[clang] [clang][NFC] Remove class layout scissor (PR #89055)
https://github.com/urnathan closed https://github.com/llvm/llvm-project/pull/89055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Remove class layout scissor (PR #89055)
https://github.com/urnathan updated
https://github.com/llvm/llvm-project/pull/89055
>From 9ab483f3451bfcaa7968c5f1cf7115144522f58a Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Mon, 1 Apr 2024 16:15:12 -0400
Subject: [PATCH 1/3] [clang] Remove class layout scissor
---
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 22 ++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 868b1ab98e048..cc51cc3476c43 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -75,7 +75,7 @@ struct CGRecordLowering {
// sentinel member type that ensures correct rounding.
struct MemberInfo {
CharUnits Offset;
-enum InfoKind { VFPtr, VBPtr, Field, Base, VBase, Scissor } Kind;
+enum InfoKind { VFPtr, VBPtr, Field, Base, VBase } Kind;
llvm::Type *Data;
union {
const FieldDecl *FD;
@@ -197,7 +197,7 @@ struct CGRecordLowering {
const CXXRecordDecl *Query) const;
void calculateZeroInit();
CharUnits calculateTailClippingOffset(bool isNonVirtualBaseType) const;
- void checkBitfieldClipping() const;
+ void checkBitfieldClipping(bool isNonVirtualBaseType) const;
/// Determines if we need a packed llvm struct.
void determinePacked(bool NVBaseType);
/// Inserts padding everywhere it's needed.
@@ -299,8 +299,8 @@ void CGRecordLowering::lower(bool NVBaseType) {
accumulateVBases();
}
llvm::stable_sort(Members);
+ checkBitfieldClipping(NVBaseType);
Members.push_back(StorageInfo(Size, getIntNType(8)));
- checkBitfieldClipping();
determinePacked(NVBaseType);
insertPadding();
Members.pop_back();
@@ -894,8 +894,6 @@ CGRecordLowering::calculateTailClippingOffset(bool
isNonVirtualBaseType) const {
}
void CGRecordLowering::accumulateVBases() {
- Members.push_back(MemberInfo(calculateTailClippingOffset(false),
- MemberInfo::Scissor, nullptr, RD));
for (const auto : RD->vbases()) {
const CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl();
if (BaseDecl->isEmpty())
@@ -950,18 +948,20 @@ void CGRecordLowering::calculateZeroInit() {
}
// Verify accumulateBitfields computed the correct storage representations.
-void CGRecordLowering::checkBitfieldClipping() const {
+void CGRecordLowering::checkBitfieldClipping(
+bool isNonVirtualBaseType LLVM_ATTRIBUTE_UNUSED) const {
#ifndef NDEBUG
+ auto ScissorOffset = calculateTailClippingOffset(isNonVirtualBaseType);
auto Tail = CharUnits::Zero();
for (const auto : Members) {
-// Only members with data and the scissor can cut into tail padding.
-if (!M.Data && M.Kind != MemberInfo::Scissor)
+// Only members with data could possibly overlap.
+if (!M.Data)
continue;
assert(M.Offset >= Tail && "Bitfield access unit is not clipped");
-Tail = M.Offset;
-if (M.Data)
- Tail += getSize(M.Data);
+Tail = M.Offset + getSize(M.Data);
+assert((Tail <= ScissorOffset || M.Offset >= ScissorOffset) &&
+ "Bitfield straddles scissor offset");
}
#endif
}
>From 050df411c74bdab8d9d6562c127abc92babbb527 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Wed, 17 Apr 2024 17:15:57 -0400
Subject: [PATCH 2/3] Fix param spelling
---
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index cc51cc3476c43..38167903cda50 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -949,9 +949,9 @@ void CGRecordLowering::calculateZeroInit() {
// Verify accumulateBitfields computed the correct storage representations.
void CGRecordLowering::checkBitfieldClipping(
-bool isNonVirtualBaseType LLVM_ATTRIBUTE_UNUSED) const {
+bool IsNonVirtualBaseType LLVM_ATTRIBUTE_UNUSED) const {
#ifndef NDEBUG
- auto ScissorOffset = calculateTailClippingOffset(isNonVirtualBaseType);
+ auto ScissorOffset = calculateTailClippingOffset(IsNonVirtualBaseType);
auto Tail = CharUnits::Zero();
for (const auto : Members) {
// Only members with data could possibly overlap.
>From 4b93593a63850f4165979a38018d6c4be23dd681 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Mon, 6 May 2024 10:28:06 -0400
Subject: [PATCH 3/3] lose attribute
---
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 38167903cda50..5169be204c14d 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -948,8 +948,7 @@ void CGRecordLowering::calculateZeroInit() {
}
// Verify accumulateBitfields computed the correct storage
[clang] [clang][NFC] Remove class layout scissor (PR #89055)
https://github.com/urnathan updated
https://github.com/llvm/llvm-project/pull/89055
>From db5e6456f26ea9b859d3ff24161d7494d58bb7e1 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Mon, 1 Apr 2024 16:15:12 -0400
Subject: [PATCH 1/3] [clang] Remove class layout scissor
---
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 22 ++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 868b1ab98e048a..cc51cc3476c438 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -75,7 +75,7 @@ struct CGRecordLowering {
// sentinel member type that ensures correct rounding.
struct MemberInfo {
CharUnits Offset;
-enum InfoKind { VFPtr, VBPtr, Field, Base, VBase, Scissor } Kind;
+enum InfoKind { VFPtr, VBPtr, Field, Base, VBase } Kind;
llvm::Type *Data;
union {
const FieldDecl *FD;
@@ -197,7 +197,7 @@ struct CGRecordLowering {
const CXXRecordDecl *Query) const;
void calculateZeroInit();
CharUnits calculateTailClippingOffset(bool isNonVirtualBaseType) const;
- void checkBitfieldClipping() const;
+ void checkBitfieldClipping(bool isNonVirtualBaseType) const;
/// Determines if we need a packed llvm struct.
void determinePacked(bool NVBaseType);
/// Inserts padding everywhere it's needed.
@@ -299,8 +299,8 @@ void CGRecordLowering::lower(bool NVBaseType) {
accumulateVBases();
}
llvm::stable_sort(Members);
+ checkBitfieldClipping(NVBaseType);
Members.push_back(StorageInfo(Size, getIntNType(8)));
- checkBitfieldClipping();
determinePacked(NVBaseType);
insertPadding();
Members.pop_back();
@@ -894,8 +894,6 @@ CGRecordLowering::calculateTailClippingOffset(bool
isNonVirtualBaseType) const {
}
void CGRecordLowering::accumulateVBases() {
- Members.push_back(MemberInfo(calculateTailClippingOffset(false),
- MemberInfo::Scissor, nullptr, RD));
for (const auto : RD->vbases()) {
const CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl();
if (BaseDecl->isEmpty())
@@ -950,18 +948,20 @@ void CGRecordLowering::calculateZeroInit() {
}
// Verify accumulateBitfields computed the correct storage representations.
-void CGRecordLowering::checkBitfieldClipping() const {
+void CGRecordLowering::checkBitfieldClipping(
+bool isNonVirtualBaseType LLVM_ATTRIBUTE_UNUSED) const {
#ifndef NDEBUG
+ auto ScissorOffset = calculateTailClippingOffset(isNonVirtualBaseType);
auto Tail = CharUnits::Zero();
for (const auto : Members) {
-// Only members with data and the scissor can cut into tail padding.
-if (!M.Data && M.Kind != MemberInfo::Scissor)
+// Only members with data could possibly overlap.
+if (!M.Data)
continue;
assert(M.Offset >= Tail && "Bitfield access unit is not clipped");
-Tail = M.Offset;
-if (M.Data)
- Tail += getSize(M.Data);
+Tail = M.Offset + getSize(M.Data);
+assert((Tail <= ScissorOffset || M.Offset >= ScissorOffset) &&
+ "Bitfield straddles scissor offset");
}
#endif
}
>From 36705e5bcdcda6983ed5a163ae8ea3e9911ad275 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Wed, 17 Apr 2024 17:15:57 -0400
Subject: [PATCH 2/3] Fix param spelling
---
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index cc51cc3476c438..38167903cda508 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -949,9 +949,9 @@ void CGRecordLowering::calculateZeroInit() {
// Verify accumulateBitfields computed the correct storage representations.
void CGRecordLowering::checkBitfieldClipping(
-bool isNonVirtualBaseType LLVM_ATTRIBUTE_UNUSED) const {
+bool IsNonVirtualBaseType LLVM_ATTRIBUTE_UNUSED) const {
#ifndef NDEBUG
- auto ScissorOffset = calculateTailClippingOffset(isNonVirtualBaseType);
+ auto ScissorOffset = calculateTailClippingOffset(IsNonVirtualBaseType);
auto Tail = CharUnits::Zero();
for (const auto : Members) {
// Only members with data could possibly overlap.
>From 2d2dcdecb0328b8d397bb14072e5750ddf20a39a Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Mon, 6 May 2024 10:28:06 -0400
Subject: [PATCH 3/3] lose attribute
---
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 38167903cda508..5169be204c14d0 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -948,8 +948,7 @@ void CGRecordLowering::calculateZeroInit() {
}
// Verify accumulateBitfields computed the correct storage
[clang] [clang][NFC] Remove class layout scissor (PR #89055)
urnathan wrote: ping? https://github.com/llvm/llvm-project/pull/89055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Remove class layout scissor (PR #89055)
https://github.com/urnathan ready_for_review https://github.com/llvm/llvm-project/pull/89055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Remove class layout scissor (PR #89055)
https://github.com/urnathan updated
https://github.com/llvm/llvm-project/pull/89055
>From db5e6456f26ea9b859d3ff24161d7494d58bb7e1 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Mon, 1 Apr 2024 16:15:12 -0400
Subject: [PATCH 1/2] [clang] Remove class layout scissor
---
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 22 ++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 868b1ab98e048a..cc51cc3476c438 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -75,7 +75,7 @@ struct CGRecordLowering {
// sentinel member type that ensures correct rounding.
struct MemberInfo {
CharUnits Offset;
-enum InfoKind { VFPtr, VBPtr, Field, Base, VBase, Scissor } Kind;
+enum InfoKind { VFPtr, VBPtr, Field, Base, VBase } Kind;
llvm::Type *Data;
union {
const FieldDecl *FD;
@@ -197,7 +197,7 @@ struct CGRecordLowering {
const CXXRecordDecl *Query) const;
void calculateZeroInit();
CharUnits calculateTailClippingOffset(bool isNonVirtualBaseType) const;
- void checkBitfieldClipping() const;
+ void checkBitfieldClipping(bool isNonVirtualBaseType) const;
/// Determines if we need a packed llvm struct.
void determinePacked(bool NVBaseType);
/// Inserts padding everywhere it's needed.
@@ -299,8 +299,8 @@ void CGRecordLowering::lower(bool NVBaseType) {
accumulateVBases();
}
llvm::stable_sort(Members);
+ checkBitfieldClipping(NVBaseType);
Members.push_back(StorageInfo(Size, getIntNType(8)));
- checkBitfieldClipping();
determinePacked(NVBaseType);
insertPadding();
Members.pop_back();
@@ -894,8 +894,6 @@ CGRecordLowering::calculateTailClippingOffset(bool
isNonVirtualBaseType) const {
}
void CGRecordLowering::accumulateVBases() {
- Members.push_back(MemberInfo(calculateTailClippingOffset(false),
- MemberInfo::Scissor, nullptr, RD));
for (const auto : RD->vbases()) {
const CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl();
if (BaseDecl->isEmpty())
@@ -950,18 +948,20 @@ void CGRecordLowering::calculateZeroInit() {
}
// Verify accumulateBitfields computed the correct storage representations.
-void CGRecordLowering::checkBitfieldClipping() const {
+void CGRecordLowering::checkBitfieldClipping(
+bool isNonVirtualBaseType LLVM_ATTRIBUTE_UNUSED) const {
#ifndef NDEBUG
+ auto ScissorOffset = calculateTailClippingOffset(isNonVirtualBaseType);
auto Tail = CharUnits::Zero();
for (const auto : Members) {
-// Only members with data and the scissor can cut into tail padding.
-if (!M.Data && M.Kind != MemberInfo::Scissor)
+// Only members with data could possibly overlap.
+if (!M.Data)
continue;
assert(M.Offset >= Tail && "Bitfield access unit is not clipped");
-Tail = M.Offset;
-if (M.Data)
- Tail += getSize(M.Data);
+Tail = M.Offset + getSize(M.Data);
+assert((Tail <= ScissorOffset || M.Offset >= ScissorOffset) &&
+ "Bitfield straddles scissor offset");
}
#endif
}
>From 36705e5bcdcda6983ed5a163ae8ea3e9911ad275 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Wed, 17 Apr 2024 17:15:57 -0400
Subject: [PATCH 2/2] Fix param spelling
---
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index cc51cc3476c438..38167903cda508 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -949,9 +949,9 @@ void CGRecordLowering::calculateZeroInit() {
// Verify accumulateBitfields computed the correct storage representations.
void CGRecordLowering::checkBitfieldClipping(
-bool isNonVirtualBaseType LLVM_ATTRIBUTE_UNUSED) const {
+bool IsNonVirtualBaseType LLVM_ATTRIBUTE_UNUSED) const {
#ifndef NDEBUG
- auto ScissorOffset = calculateTailClippingOffset(isNonVirtualBaseType);
+ auto ScissorOffset = calculateTailClippingOffset(IsNonVirtualBaseType);
auto Tail = CharUnits::Zero();
for (const auto : Members) {
// Only members with data could possibly overlap.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Remove class layout scissor (PR #89055)
https://github.com/urnathan created
https://github.com/llvm/llvm-project/pull/89055
Now that `accumulateBitfields` does the correct clipping, we don't need the
scissor any more -- `checkBitfieldClipping` can compute its location directly.
>From db5e6456f26ea9b859d3ff24161d7494d58bb7e1 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Mon, 1 Apr 2024 16:15:12 -0400
Subject: [PATCH] [clang] Remove class layout scissor
---
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 22 ++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 868b1ab98e048a..cc51cc3476c438 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -75,7 +75,7 @@ struct CGRecordLowering {
// sentinel member type that ensures correct rounding.
struct MemberInfo {
CharUnits Offset;
-enum InfoKind { VFPtr, VBPtr, Field, Base, VBase, Scissor } Kind;
+enum InfoKind { VFPtr, VBPtr, Field, Base, VBase } Kind;
llvm::Type *Data;
union {
const FieldDecl *FD;
@@ -197,7 +197,7 @@ struct CGRecordLowering {
const CXXRecordDecl *Query) const;
void calculateZeroInit();
CharUnits calculateTailClippingOffset(bool isNonVirtualBaseType) const;
- void checkBitfieldClipping() const;
+ void checkBitfieldClipping(bool isNonVirtualBaseType) const;
/// Determines if we need a packed llvm struct.
void determinePacked(bool NVBaseType);
/// Inserts padding everywhere it's needed.
@@ -299,8 +299,8 @@ void CGRecordLowering::lower(bool NVBaseType) {
accumulateVBases();
}
llvm::stable_sort(Members);
+ checkBitfieldClipping(NVBaseType);
Members.push_back(StorageInfo(Size, getIntNType(8)));
- checkBitfieldClipping();
determinePacked(NVBaseType);
insertPadding();
Members.pop_back();
@@ -894,8 +894,6 @@ CGRecordLowering::calculateTailClippingOffset(bool
isNonVirtualBaseType) const {
}
void CGRecordLowering::accumulateVBases() {
- Members.push_back(MemberInfo(calculateTailClippingOffset(false),
- MemberInfo::Scissor, nullptr, RD));
for (const auto : RD->vbases()) {
const CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl();
if (BaseDecl->isEmpty())
@@ -950,18 +948,20 @@ void CGRecordLowering::calculateZeroInit() {
}
// Verify accumulateBitfields computed the correct storage representations.
-void CGRecordLowering::checkBitfieldClipping() const {
+void CGRecordLowering::checkBitfieldClipping(
+bool isNonVirtualBaseType LLVM_ATTRIBUTE_UNUSED) const {
#ifndef NDEBUG
+ auto ScissorOffset = calculateTailClippingOffset(isNonVirtualBaseType);
auto Tail = CharUnits::Zero();
for (const auto : Members) {
-// Only members with data and the scissor can cut into tail padding.
-if (!M.Data && M.Kind != MemberInfo::Scissor)
+// Only members with data could possibly overlap.
+if (!M.Data)
continue;
assert(M.Offset >= Tail && "Bitfield access unit is not clipped");
-Tail = M.Offset;
-if (M.Data)
- Tail += getSize(M.Data);
+Tail = M.Offset + getSize(M.Data);
+assert((Tail <= ScissorOffset || M.Offset >= ScissorOffset) &&
+ "Bitfield straddles scissor offset");
}
#endif
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Move tailclipping to bitfield allocation (PR #87090)
https://github.com/urnathan closed https://github.com/llvm/llvm-project/pull/87090 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Move tailclipping to bitfield allocation (PR #87090)
urnathan wrote: @rjmccall can you take a look at this? as the description says, this turns out to not be NFC, but important on (probably only) LP64 machines. https://github.com/llvm/llvm-project/pull/87090 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Adjust TBAA Base Info API (PR #73263)
https://github.com/urnathan closed https://github.com/llvm/llvm-project/pull/73263 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix bitfield access unit for vbase corner case (PR #87238)
https://github.com/urnathan closed https://github.com/llvm/llvm-project/pull/87238 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix bitfield access unit for vbase corner case (PR #87238)
https://github.com/urnathan ready_for_review https://github.com/llvm/llvm-project/pull/87238 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] MIPS/Clang: Set HasUnalignedAccess false if +strict-align (PR #87257)
@@ -330,6 +331,8 @@ class LLVM_LIBRARY_VISIBILITY MipsTargetInfo : public
TargetInfo {
IsMicromips = true;
else if (Feature == "+mips32r6" || Feature == "+mips64r6")
HasUnalignedAccess = true;
+ else if (Feature == "+strict-align")
+StrictAlign = true;
urnathan wrote:
Why not clear HasUnalignedAccess directly here? That's what AArch64 and ARM do.
https://github.com/llvm/llvm-project/pull/87257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Better bitfield access units (PR #65742)
urnathan wrote: Be aware of bug #87227 and PR #87238 to address that https://github.com/llvm/llvm-project/pull/65742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Move tailclipping to bitfield allocation (PR #87090)
https://github.com/urnathan converted_to_draft https://github.com/llvm/llvm-project/pull/87090 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix bitfield access unit for vbase corner case (PR #87238)
https://github.com/urnathan created
https://github.com/llvm/llvm-project/pull/87238
This fixes #87227 My change to bitfield access unit allocation (#65742) causes
an ICE for a corner case of vbase allocation: a class where an unshared (i.e.
not the nearly-empty base optimization) vbase is placed below nvsize.
Unfortunately, although there was a testcase for such a class layout, it didn't
have the immediately preceding bitfield -- the reason the scissor needs to be
correct.
The fix is to break out the scissor calculation from `accumulateVbases`, and
have `allocateBitfields` be aware it is creating either the base subobject, or
the complete object. Then it can call the scissor calculator to get the
appropriate upper bound. The scissor calculation can cause a base walk, I
thought it best to cache the result in `allocateBitfields`, as we can reach
that point multiple times with unfortunately-sized bitfield spans.
In breaking out the scissor calculation, I discovered a couple more member fns
that could be const qualified -- as before do you want that as a separate PR?
>From 3b0bfce77d6d70bc35316484b39bb83123b8a583 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Sun, 31 Mar 2024 11:11:13 -0400
Subject: [PATCH] [clang] Fix bitfield access unit for vbase corner case
We must account for unshared vbases that are allocated below nvsize.
---
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 57 +++---
.../test/CodeGenCXX/bitfield-access-tail.cpp | 104 --
2 files changed, 113 insertions(+), 48 deletions(-)
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index e32023aeac1e6f..634a55fec5182e 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -185,9 +185,10 @@ struct CGRecordLowering {
/// Lowers an ASTRecordLayout to a llvm type.
void lower(bool NonVirtualBaseType);
void lowerUnion(bool isNoUniqueAddress);
- void accumulateFields();
+ void accumulateFields(bool isNonVirtualBaseType);
RecordDecl::field_iterator
- accumulateBitFields(RecordDecl::field_iterator Field,
+ accumulateBitFields(bool isNonVirtualBaseType,
+ RecordDecl::field_iterator Field,
RecordDecl::field_iterator FieldEnd);
void computeVolatileBitfields();
void accumulateBases();
@@ -195,8 +196,10 @@ struct CGRecordLowering {
void accumulateVBases();
/// Recursively searches all of the bases to find out if a vbase is
/// not the primary vbase of some base class.
- bool hasOwnStorage(const CXXRecordDecl *Decl, const CXXRecordDecl *Query);
+ bool hasOwnStorage(const CXXRecordDecl *Decl,
+ const CXXRecordDecl *Query) const;
void calculateZeroInit();
+ CharUnits calculateTailClippingOffset(bool isNonVirtualBaseType) const;
/// Lowers bitfield storage types to I8 arrays for bitfields with tail
/// padding that is or can potentially be used.
void clipTailPadding();
@@ -287,7 +290,7 @@ void CGRecordLowering::lower(bool NVBaseType) {
computeVolatileBitfields();
return;
}
- accumulateFields();
+ accumulateFields(NVBaseType);
// RD implies C++.
if (RD) {
accumulateVPtrs();
@@ -378,12 +381,12 @@ void CGRecordLowering::lowerUnion(bool isNoUniqueAddress)
{
Packed = true;
}
-void CGRecordLowering::accumulateFields() {
+void CGRecordLowering::accumulateFields(bool isNonVirtualBaseType) {
for (RecordDecl::field_iterator Field = D->field_begin(),
FieldEnd = D->field_end();
Field != FieldEnd;) {
if (Field->isBitField()) {
- Field = accumulateBitFields(Field, FieldEnd);
+ Field = accumulateBitFields(isNonVirtualBaseType, Field, FieldEnd);
assert((Field == FieldEnd || !Field->isBitField()) &&
"Failed to accumulate all the bitfields");
} else if (Field->isZeroSize(Context)) {
@@ -404,9 +407,12 @@ void CGRecordLowering::accumulateFields() {
}
// Create members for bitfields. Field is a bitfield, and FieldEnd is the end
-// iterator of the record. Return the first non-bitfield encountered.
+// iterator of the record. Return the first non-bitfield encountered. We need
+// to know whether this is the base or complete layout, as virtual bases could
+// affect the upper bound of bitfield access unit allocation.
RecordDecl::field_iterator
-CGRecordLowering::accumulateBitFields(RecordDecl::field_iterator Field,
+CGRecordLowering::accumulateBitFields(bool isNonVirtualBaseType,
+ RecordDecl::field_iterator Field,
RecordDecl::field_iterator FieldEnd) {
if (isDiscreteBitFieldABI()) {
// Run stores the first element of the current run of bitfields. FieldEnd
is
@@ -505,6 +511,10 @@
CGRecordLowering::accumulateBitFields(RecordDecl::field_iterator Field,
[clang] [llvm] MIPS: Support -m(no-)unaligned-access for r6 (PR #85174)
urnathan wrote: #65742 is committed, so MIPS' TargetInfo will need adjusting to propagate the unaligned capability. See 7df79ababee8 https://github.com/llvm/llvm-project/pull/85174 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Move tailclipping to bitfield allocation (PR #87090)
urnathan wrote: Sigh, clipTailPadding is still needed, because of no_unique_address and empty base placement. Will update. https://github.com/llvm/llvm-project/pull/87090 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Move tailclipping to bitfield allocation (PR #87090)
https://github.com/urnathan created
https://github.com/llvm/llvm-project/pull/87090
This follows on from PR65742. As I suspected clipTailPadding's functionality
can be moved into accumulateBitfields; it has sufficient information now. It
turns out that clipTailPadding was undoing some of the work just added to
accumulateBitfields. Specifically, on a strict-alignment LP64 machine,
accumulateBitfields could determine that a 24-bit access unit was the best unit
to use (loading 3 separate bytes being better than 4). But if those 3 bytes
were followed by a padding byte, clipTailPadding would not make the storage a
3-byte array, and the access unit would be an (unaligned) i32. Boo! The new
testcase shows this -- it's written to work on ILP32 and LP64, but the common
case I think would be on LP64 where a pointer follows the bitfields, and
there's 4 bytes of alignment padding between.
The fine-grained-bitfield accesses needed adjusting, to preserve the existing
behaviour of extending them into useable tail padding. You'll see we now check
this later -- allowing it to grow, but preventing accumulation of a subsequent
span. I think that's in keeping with the desired behaviour of that option.
>From 4d9861a9ca23d8de7b33c4b830b5c6f78e8a3768 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Sun, 24 Mar 2024 09:37:46 -0400
Subject: [PATCH 1/2] Move bitfield clipping into bitfield accumulation
---
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 63 -
clang/test/CodeGen/bitfield-access-unit.c | 18 ++
2 files changed, 55 insertions(+), 26 deletions(-)
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index e32023aeac1e6f..5b9045f661e048 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -41,10 +41,11 @@ namespace {
/// contains enough information to determine where the runs break. Microsoft
/// and Itanium follow different rules and use different codepaths.
/// * It is desired that, when possible, bitfields use the appropriate iN type
-/// when lowered to llvm types. For example unsigned x : 24 gets lowered to
+/// when lowered to llvm types. For example unsigned x : 24 gets lowered to
/// i24. This isn't always possible because i24 has storage size of 32 bit
-/// and if it is possible to use that extra byte of padding we must use
-/// [i8 x 3] instead of i24. The function clipTailPadding does this.
+/// and if it is possible to use that extra byte of padding we must use [i8 x
+/// 3] instead of i24. This is computed when accumulating bitfields in
+/// accumulateBitfields.
/// C++ examples that require clipping:
/// struct { int a : 24; char b; }; // a must be clipped, b goes at offset 3
/// struct A { int a : 24; ~A(); }; // a must be clipped because:
@@ -197,9 +198,7 @@ struct CGRecordLowering {
/// not the primary vbase of some base class.
bool hasOwnStorage(const CXXRecordDecl *Decl, const CXXRecordDecl *Query);
void calculateZeroInit();
- /// Lowers bitfield storage types to I8 arrays for bitfields with tail
- /// padding that is or can potentially be used.
- void clipTailPadding();
+ void checkTailPadding();
/// Determines if we need a packed llvm struct.
void determinePacked(bool NVBaseType);
/// Inserts padding everywhere it's needed.
@@ -302,7 +301,7 @@ void CGRecordLowering::lower(bool NVBaseType) {
}
llvm::stable_sort(Members);
Members.push_back(StorageInfo(Size, getIntNType(8)));
- clipTailPadding();
+ checkTailPadding();
determinePacked(NVBaseType);
insertPadding();
Members.pop_back();
@@ -521,6 +520,7 @@
CGRecordLowering::accumulateBitFields(RecordDecl::field_iterator Field,
// available padding characters.
RecordDecl::field_iterator BestEnd = Begin;
CharUnits BestEndOffset;
+ bool BestClipped; // Whether the representation must be in a byte array.
for (;;) {
// AtAlignedBoundary is true iff Field is the (potential) start of a new
@@ -583,10 +583,9 @@
CGRecordLowering::accumulateBitFields(RecordDecl::field_iterator Field,
// this is the best seen so far.
BestEnd = Field;
BestEndOffset = BeginOffset + AccessSize;
-if (Types.getCodeGenOpts().FineGrainedBitfieldAccesses)
- // Fine-grained access, so no merging of spans.
- InstallBest = true;
-else if (!BitSizeSinceBegin)
+// Assume clipped until proven not below.
+BestClipped = true;
+if (!BitSizeSinceBegin)
// A zero-sized initial span -- this will install nothing and reset
// for another.
InstallBest = true;
@@ -614,6 +613,12 @@
CGRecordLowering::accumulateBitFields(RecordDecl::field_iterator Field,
// The access unit is not at a naturally aligned offset within the
// structure.
InstallBest = true;
+
+ if (InstallBest && BestEnd ==
[clang] [clang] Better bitfield access units (PR #65742)
urnathan wrote: Committed, expect tailclipping cleanup PR soon https://github.com/llvm/llvm-project/pull/65742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Better bitfield access units (PR #65742)
https://github.com/urnathan closed https://github.com/llvm/llvm-project/pull/65742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 7df79ab - [clang] TargetInfo hook for unaligned bitfields (#65742)
Author: Nathan Sidwell
Date: 2024-03-29T09:35:31-04:00
New Revision: 7df79ababee8d03b27bbaba1aabc2ec4ea14143e
URL:
https://github.com/llvm/llvm-project/commit/7df79ababee8d03b27bbaba1aabc2ec4ea14143e
DIFF:
https://github.com/llvm/llvm-project/commit/7df79ababee8d03b27bbaba1aabc2ec4ea14143e.diff
LOG: [clang] TargetInfo hook for unaligned bitfields (#65742)
Promote ARM & AArch64's HasUnaligned to TargetInfo and set for all
targets.
Added:
Modified:
clang/include/clang/Basic/TargetInfo.h
clang/lib/Basic/TargetInfo.cpp
clang/lib/Basic/Targets/AArch64.cpp
clang/lib/Basic/Targets/AArch64.h
clang/lib/Basic/Targets/ARM.cpp
clang/lib/Basic/Targets/ARM.h
clang/lib/Basic/Targets/LoongArch.cpp
clang/lib/Basic/Targets/LoongArch.h
clang/lib/Basic/Targets/Mips.h
clang/lib/Basic/Targets/PPC.h
clang/lib/Basic/Targets/SystemZ.h
clang/lib/Basic/Targets/VE.h
clang/lib/Basic/Targets/WebAssembly.h
clang/lib/Basic/Targets/X86.h
Removed:
diff --git a/clang/include/clang/Basic/TargetInfo.h
b/clang/include/clang/Basic/TargetInfo.h
index 374595edd2ce4a..e1ef7454f01669 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -267,6 +267,9 @@ class TargetInfo : public TransferrableTargetInfo,
LLVM_PREFERRED_TYPE(bool)
unsigned AllowAMDGPUUnsafeFPAtomics : 1;
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned HasUnalignedAccess : 1;
+
unsigned ARMCDECoprocMask : 8;
unsigned MaxOpenCLWorkGroupSize;
@@ -859,6 +862,18 @@ class TargetInfo : public TransferrableTargetInfo,
return PointerWidth;
}
+ /// Return true iff unaligned accesses are a single instruction (rather than
+ /// a synthesized sequence).
+ bool hasUnalignedAccess() const { return HasUnalignedAccess; }
+
+ /// Return true iff unaligned accesses are cheap. This affects placement and
+ /// size of bitfield loads/stores. (Not the ABI-mandated placement of
+ /// the bitfields themselves.)
+ bool hasCheapUnalignedBitFieldAccess() const {
+// Simply forward to the unaligned access getter.
+return hasUnalignedAccess();
+ }
+
/// \brief Returns the default value of the __USER_LABEL_PREFIX__ macro,
/// which is the prefix given to user symbols by default.
///
diff --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp
index 5d9055174c089a..f96956f31d50dc 100644
--- a/clang/lib/Basic/TargetInfo.cpp
+++ b/clang/lib/Basic/TargetInfo.cpp
@@ -157,6 +157,7 @@ TargetInfo::TargetInfo(const llvm::Triple ) : Triple(T) {
HasAArch64SVETypes = false;
HasRISCVVTypes = false;
AllowAMDGPUUnsafeFPAtomics = false;
+ HasUnalignedAccess = false;
ARMCDECoprocMask = 0;
// Default to no types using fpret.
diff --git a/clang/lib/Basic/Targets/AArch64.cpp
b/clang/lib/Basic/Targets/AArch64.cpp
index 1c3199bd76eed6..1569b5e04b770a 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -188,6 +188,8 @@ AArch64TargetInfo::AArch64TargetInfo(const llvm::Triple
,
assert(UseBitFieldTypeAlignment && "bitfields affect type alignment");
UseZeroLengthBitfieldAlignment = true;
+ HasUnalignedAccess = true;
+
// AArch64 targets default to using the ARM C++ ABI.
TheCXXABI.set(TargetCXXABI::GenericAArch64);
@@ -496,7 +498,7 @@ void AArch64TargetInfo::getTargetDefines(const LangOptions
,
if (HasPAuthLR)
Builder.defineMacro("__ARM_FEATURE_PAUTH_LR", "1");
- if (HasUnaligned)
+ if (HasUnalignedAccess)
Builder.defineMacro("__ARM_FEATURE_UNALIGNED", "1");
if ((FPU & NeonMode) && HasFullFP16)
@@ -921,7 +923,8 @@ bool
AArch64TargetInfo::handleTargetFeatures(std::vector ,
HasSM4 = true;
}
if (Feature == "+strict-align")
- HasUnaligned = false;
+ HasUnalignedAccess = false;
+
// All predecessor archs are added but select the latest one for ArchKind.
if (Feature == "+v8a" && ArchInfo->Version < llvm::AArch64::ARMV8A.Version)
ArchInfo = ::AArch64::ARMV8A;
diff --git a/clang/lib/Basic/Targets/AArch64.h
b/clang/lib/Basic/Targets/AArch64.h
index 542894c66412dc..12fb50286f7511 100644
--- a/clang/lib/Basic/Targets/AArch64.h
+++ b/clang/lib/Basic/Targets/AArch64.h
@@ -38,7 +38,6 @@ class LLVM_LIBRARY_VISIBILITY AArch64TargetInfo : public
TargetInfo {
bool HasSHA2 = false;
bool HasSHA3 = false;
bool HasSM4 = false;
- bool HasUnaligned = true;
bool HasFullFP16 = false;
bool HasDotProd = false;
bool HasFP16FML = false;
diff --git a/clang/lib/Basic/Targets/ARM.cpp b/clang/lib/Basic/Targets/ARM.cpp
index 55b71557452fa0..877799c66ec4f2 100644
--- a/clang/lib/Basic/Targets/ARM.cpp
+++ b/clang/lib/Basic/Targets/ARM.cpp
@@ -509,7 +509,7 @@ bool
ARMTargetInfo::handleTargetFeatures(std::vector ,
SHA2 = 0;
AES = 0;
DSP = 0;
- Unaligned = 1;
+ HasUnalignedAccess = true;
SoftFloat = false;
// Note that
[clang] a8ca4ab - [clang][NFC] Bitfield access unit tests (#65742)
Author: Nathan Sidwell
Date: 2024-03-29T09:35:31-04:00
New Revision: a8ca4abfcfa98d28ec46ee497e1fc5e91f8e1ad6
URL:
https://github.com/llvm/llvm-project/commit/a8ca4abfcfa98d28ec46ee497e1fc5e91f8e1ad6
DIFF:
https://github.com/llvm/llvm-project/commit/a8ca4abfcfa98d28ec46ee497e1fc5e91f8e1ad6.diff
LOG: [clang][NFC] Bitfield access unit tests (#65742)
Verify bitfield access units.
Added:
clang/test/CodeGen/aapcs-bitfield-access-unit.c
clang/test/CodeGen/bitfield-access-pad.c
clang/test/CodeGen/bitfield-access-unit.c
clang/test/CodeGenCXX/bitfield-access-empty.cpp
clang/test/CodeGenCXX/bitfield-access-tail.cpp
clang/test/CodeGenCXX/bitfield-ir.cpp
Modified:
clang/test/CodeGen/arm-bitfield-alignment.c
clang/test/CodeGen/arm64-be-bitfield.c
clang/test/CodeGen/no-bitfield-type-align.c
clang/test/CodeGen/struct-x86-darwin.c
clang/test/CodeGenCXX/bitfield.cpp
Removed:
diff --git a/clang/test/CodeGen/aapcs-bitfield-access-unit.c
b/clang/test/CodeGen/aapcs-bitfield-access-unit.c
new file mode 100644
index 00..ff28397c529007
--- /dev/null
+++ b/clang/test/CodeGen/aapcs-bitfield-access-unit.c
@@ -0,0 +1,231 @@
+// RUN: %clang_cc1 -triple armv8-none-linux-eabi -fno-aapcs-bitfield-width
-fdump-record-layouts-simple -emit-llvm -o /dev/null %s | FileCheck %s
-check-prefixes=LAYOUT
+// RUN: %clang_cc1 -triple armebv8-none-linux-eabi -fno-aapcs-bitfield-width
-fdump-record-layouts-simple -emit-llvm -o /dev/null %s | FileCheck %s
-check-prefixes=LAYOUT
+
+// RUN: %clang_cc1 -triple armv8-none-linux-eabi -faapcs-bitfield-width
-fdump-record-layouts-simple -emit-llvm -o /dev/null %s | FileCheck %s
-check-prefixes=LAYOUT
+// RUN: %clang_cc1 -triple armebv8-none-linux-eabi -faapcs-bitfield-width
-fdump-record-layouts-simple -emit-llvm -o /dev/null %s | FileCheck %s
-check-prefixes=LAYOUT
+
+struct st0 {
+ short c : 7;
+} st0;
+// LAYOUT-LABEL: LLVMType:%struct.st0 =
+// LAYOUT-SAME: type { i8, i8 }
+// LAYOUT: BitFields:[
+// LAYOUT-NEXT:
+
+struct st1 {
+ int a : 10;
+ short c : 6;
+} st1;
+// LAYOUT-LABEL: LLVMType:%struct.st1 =
+// LAYOUT-SAME: type { i16, [2 x i8] }
+// LAYOUT: BitFields:[
+// LAYOUT-NEXT:
+
+struct st2 {
+ int a : 10;
+ short c : 7;
+} st2;
+// LAYOUT-LABEL: LLVMType:%struct.st2 =
+// LAYOUT-SAME: type { i16, i8 }
+// LAYOUT: BitFields:[
+// LAYOUT-NEXT:
+
+struct st3 {
+ volatile short c : 7;
+} st3;
+// LAYOUT-LABEL: LLVMType:%struct.st3 =
+// LAYOUT-SAME: type { i8, i8 }
+// LAYOUT: BitFields:[
+// LAYOUT-NEXT:
+
+struct st4 {
+ int b : 9;
+ volatile char c : 5;
+} st4;
+// LAYOUT-LABEL: LLVMType:%struct.st4 =
+// LAYOUT-SAME: type { i16, [2 x i8] }
+// LAYOUT: BitFields:[
+// LAYOUT-NEXT:
+
+struct st5 {
+ int a : 12;
+ volatile char c : 5;
+} st5;
+// LAYOUT-LABEL: LLVMType:%struct.st5 =
+// LAYOUT-SAME: type { i16, i8 }
+// LAYOUT: BitFields:[
+// LAYOUT-NEXT:
+
+struct st6 {
+ int a : 12;
+ char b;
+ int c : 5;
+} st6;
+// LAYOUT-LABEL: LLVMType:%struct.st6 =
+// LAYOUT-SAME: type { i16, i8, i8 }
+// LAYOUT: BitFields:[
+// LAYOUT-NEXT:
+
+struct st7a {
+ char a;
+ int b : 5;
+} st7a;
+// LAYOUT-LABEL: LLVMType:%struct.st7a =
+// LAYOUT-SAME: type { i8, i8, [2 x i8] }
+// LAYOUT: BitFields:[
+// LAYOUT-NEXT:
+
+struct st7b {
+ char x;
+ volatile struct st7a y;
+} st7b;
+// LAYOUT-LABEL: LLVMType:%struct.st7b =
+// LAYOUT-SAME: type { i8, [3 x i8], %struct.st7a }
+// LAYOUT: BitFields:[
+// LAYOUT-NEXT: ]>
+
+struct st8 {
+ unsigned f : 16;
+} st8;
+// LAYOUT-LABEL: LLVMType:%struct.st8 =
+// LAYOUT-SAME: type { i16, [2 x i8] }
+// LAYOUT: BitFields:[
+// LAYOUT-NEXT:
+
+struct st9{
+ int f : 8;
+} st9;
+// LAYOUT-LABEL: LLVMType:%struct.st9 =
+// LAYOUT-SAME: type { i8, [3 x i8] }
+// LAYOUT: BitFields:[
+// LAYOUT-NEXT:
+
+struct st10{
+ int e : 1;
+ int f : 8;
+} st10;
+// LAYOUT-LABEL: LLVMType:%struct.st10 =
+// LAYOUT-SAME: type { i16, [2 x i8] }
+// LAYOUT: BitFields:[
+// LAYOUT-NEXT:
+
+struct st11{
+ char e;
+ int f : 16;
+} st11;
+// LAYOUT-LABEL: LLVMType:%struct.st11 =
+// LAYOUT-SAME: type <{ i8, i16, i8 }>
+// LAYOUT: BitFields:[
+// LAYOUT-NEXT:
+
+struct st12{
+ int e : 8;
+ int f : 16;
+} st12;
+// LAYOUT-LABEL: LLVMType:%struct.st12 =
+// LAYOUT-SAME: type { i24 }
+// LAYOUT: BitFields:[
+// LAYOUT-NEXT:
+
+struct st13 {
+ char a : 8;
+ int b : 32;
+} __attribute__((packed)) st13;
+// LAYOUT-LABEL: LLVMType:%struct.st13 =
+// LAYOUT-SAME: type { [5 x i8] }
+// LAYOUT: BitFields:[
+// LAYOUT-NEXT:
+
+struct st14 {
+ char a : 8;
+} __attribute__((packed)) st14;
+// LAYOUT-LABEL: LLVMType:%struct.st14 =
+// LAYOUT-SAME: type { i8 }
+// LAYOUT: BitFields:[
+// LAYOUT-NEXT:
+
+struct st15 {
+ short a : 8;
+} __attribute__((packed)) st15;
+// LAYOUT-LABEL: LLVMType:%struct.st15 =
+// LAYOUT-SAME: type { i8 }
+// LAYOUT: BitFields:[
+// LAYOUT-NEXT:
+
+struct st16 {
+ int a
[clang] [clang] Better bitfield access units (PR #65742)
urnathan wrote: @rjmccall thanks, here is an update with those changes. I've collapsed it to the 3 commits mentioned earlier 1) Tests marked up for the current behaviour 2) TargetInfo strict/flexible alignment load predicate 3) The new algorithm https://github.com/llvm/llvm-project/pull/65742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Better bitfield access units (PR #65742)
@@ -439,82 +444,247 @@
CGRecordLowering::accumulateBitFields(RecordDecl::field_iterator Field,
Members.push_back(MemberInfo(bitsToCharUnits(StartBitOffset),
MemberInfo::Field, nullptr, *Field));
}
-return;
+return Field;
}
- // Check if OffsetInRecord (the size in bits of the current run) is better
- // as a single field run. When OffsetInRecord has legal integer width, and
- // its bitfield offset is naturally aligned, it is better to make the
- // bitfield a separate storage component so as it can be accessed directly
- // with lower cost.
- auto IsBetterAsSingleFieldRun = [&](uint64_t OffsetInRecord,
- uint64_t StartBitOffset) {
-if (!Types.getCodeGenOpts().FineGrainedBitfieldAccesses)
- return false;
-if (OffsetInRecord < 8 || !llvm::isPowerOf2_64(OffsetInRecord) ||
-!DataLayout.fitsInLegalInteger(OffsetInRecord))
- return false;
-// Make sure StartBitOffset is naturally aligned if it is treated as an
-// IType integer.
-if (StartBitOffset %
-Context.toBits(getAlignment(getIntNType(OffsetInRecord))) !=
-0)
- return false;
-return true;
- };
+ // The SysV ABI can overlap bitfield storage units with both other bitfield
+ // storage units /and/ other non-bitfield data members. Accessing a sequence
+ // of bitfields mustn't interfere with adjacent non-bitfields -- they're
+ // permitted to be accessed in separate threads for instance.
+
+ // We split runs of bit-fields into a sequence of "access units". When we
emit
+ // a load or store of a bit-field, we'll load/store the entire containing
+ // access unit. As mentioned, the standard requires that these loads and
+ // stores must not interfere with accesses to other memory locations, and it
+ // defines the bit-field's memory location as the current run of
+ // non-zero-width bit-fields. So an access unit must never overlap with
+ // non-bit-field storage or cross a zero-width bit-field. Otherwise, we're
+ // free to draw the lines as we see fit.
+
+ // Drawing these lines well can be complicated. LLVM generally can't modify a
+ // program to access memory that it didn't before, so using very narrow
access
+ // units can prevent the compiler from using optimal access patterns. For
+ // example, suppose a run of bit-fields occupies four bytes in a struct. If
we
+ // split that into four 1-byte access units, then a sequence of assignments
+ // that doesn't touch all four bytes may have to be emitted with multiple
+ // 8-bit stores instead of a single 32-bit store. On the other hand, if we
use
+ // very wide access units, we may find ourselves emitting accesses to
+ // bit-fields we didn't really need to touch, just because LLVM was unable to
+ // clean up after us.
+
+ // It is desirable to have access units be aligned powers of 2 no larger than
+ // a register. (On non-strict alignment ISAs, the alignment requirement can
be
+ // dropped.) A three byte access unit will be accessed using 2-byte and
1-byte
+ // accesses and bit manipulation. If no bitfield straddles across the two
+ // separate accesses, it is better to have separate 2-byte and 1-byte access
+ // units, as then LLVM will not generate unnecessary memory accesses, or bit
+ // manipulation. Similarly, on a strict-alignment architecture, it is better
+ // to keep access-units naturally aligned, to avoid similar bit
+ // manipulation synthesizing larger unaligned accesses.
+
+ // Bitfields that share parts of a single byte are, of necessity, placed in
+ // the same access unit. That unit will encompass a consecutive run where
+ // adjacent bitfields share parts of a byte. (The first bitfield of such an
+ // access unit will start at the beginning of a byte.)
+
+ // We then try and accumulate adjacent access units when the combined unit is
+ // naturally sized, no larger than a register, and (on a strict alignment
+ // ISA), naturally aligned. Note that this requires lookahead to one or more
+ // subsequent access units. For instance, consider a 2-byte access-unit
+ // followed by 2 1-byte units. We can merge that into a 4-byte access-unit,
+ // but we would not want to merge a 2-byte followed by a single 1-byte (and
no
+ // available tail padding). We keep track of the best access unit seen so
far,
+ // and use that when we determine we cannot accumulate any more. Then we
start
+ // again at the bitfield following that best one.
+
+ // The accumulation is also prevented when:
+ // *) it would cross a character-aigned zero-width bitfield, or
+ // *) fine-grained bitfield access option is in effect.
+
+ CharUnits RegSize =
+ bitsToCharUnits(Context.getTargetInfo().getRegisterWidth());
+ unsigned CharBits = Context.getCharWidth();
+
+ // Data about the start of the span we're accumulating to create an access
+ // unit from. Begin is the first
[clang] [clang] Better bitfield access units (PR #65742)
urnathan wrote: Sorry to push another update, but I realized the LimitOffset computation could be sunk to the point of use, and therefore not computed in all cases. https://github.com/llvm/llvm-project/pull/65742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Better bitfield access units (PR #65742)
urnathan wrote:
@rjmccall here is a rebase an update, which I think addresses all your
comments. I did make some material changes though:
1) I removed the Volatile handling. I was always a little uncomfortable with it
because it didn't affect the access units of a non-volatile bitfield that ends
up being volatile qualified via the structure's volatile quals itself. Users of
volatile fields are probably confused, but if not they have a std-blessed
mechanism for isolating access units -- a zero-length bitfield. The heuristic
broke the idea that this patch /just/ implemented a better access algorithm so
I was being inconsistent. AFAICT the ARM ABI, which seems to be one that
describes volatile bitfields carefully, does not specify that volatile
bitfields should be as isolated as possible. This change causes one change,
in `CodeGen/aapcs-bitfield.c` with:
```
struct st5 {
int a : 12;
volatile char c : 5;
} st5;
```
The two bitfields are placed into a single 32-bit access unit, rather than
separate 16bit ones. Either behaviour is ok with the aapcs. (The previous
algorithm would have placed them into a single 24bit field if they abutted at a
byte boundary, no change in that behaviour now.)
2) Implementing the barrier behaviour you wanted. I tried to find a psABI that
had the right attributes to place barriers at arbitrary bit positions, to see
if it had any comment, but failed to find one. as you say, this simplifies
things, but ...
3) The barrier bitfield behaviour that we already had showed an interesting
behaviour, which I suspect is from a later scissoring processing. Namely with:
```
struct A {
unsigned a : 16;
unsigned b : 8;
char : 0;
unsigned d;
} a;
```
we'd generate an llvm struct of `{ i24, i32}`, but then adjust the access unit
for the bitfields to be a 32-bit unit itself. That seems conformant, because
nothing else uses that 4th byte, and the std merely says that the bitfield
starts at an allocation unit boundary. This patch was originally treating that
barrier as a limit to the current span, whereas we can use the next member with
storage as that limit. This is actually the behaviour we had when we reached
the end of a run of bitfields, so I have made that change as you can see from
the changed handling setting Barrier.
4) I adjusted the new testcases to reduce their complexity -- we don't care
about endianness, which only affects the placement of the bitfields within
their access unit, not the access units themselves.
It may be that the later pass that was adjusting bitfield accesses to natural
sizes can be simplified or deleted. I've not investigated.
https://github.com/llvm/llvm-project/pull/65742
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Better bitfield access units (PR #65742)
@@ -439,82 +444,194 @@
CGRecordLowering::accumulateBitFields(RecordDecl::field_iterator Field,
Members.push_back(MemberInfo(bitsToCharUnits(StartBitOffset),
MemberInfo::Field, nullptr, *Field));
}
-return;
+return Field;
}
- // Check if OffsetInRecord (the size in bits of the current run) is better
- // as a single field run. When OffsetInRecord has legal integer width, and
- // its bitfield offset is naturally aligned, it is better to make the
- // bitfield a separate storage component so as it can be accessed directly
- // with lower cost.
- auto IsBetterAsSingleFieldRun = [&](uint64_t OffsetInRecord,
- uint64_t StartBitOffset) {
-if (!Types.getCodeGenOpts().FineGrainedBitfieldAccesses)
- return false;
-if (OffsetInRecord < 8 || !llvm::isPowerOf2_64(OffsetInRecord) ||
-!DataLayout.fitsInLegalInteger(OffsetInRecord))
- return false;
-// Make sure StartBitOffset is naturally aligned if it is treated as an
-// IType integer.
-if (StartBitOffset %
-Context.toBits(getAlignment(getIntNType(OffsetInRecord))) !=
-0)
- return false;
-return true;
- };
+ // The SysV ABI can overlap bitfield storage units with both other bitfield
+ // storage units /and/ other non-bitfield data members. Accessing a sequence
+ // of bitfields mustn't interfere with adjacent non-bitfields -- they're
+ // permitted to be accessed in separate threads for instance.
+
+ // We split runs of bit-fields into a sequence of "access units". When we
emit
+ // a load or store of a bit-field, we'll load/store the entire containing
+ // access unit. As mentioned, the standard requires that these loads and
+ // stores must not interfere with accesses to other memory locations, and it
+ // defines the bit-field's memory location as the current run of
+ // non-zero-width bit-fields. So an access unit must never overlap with
+ // non-bit-field storage or cross a zero-width bit-field. Otherwise, we're
+ // free to draw the lines as we see fit.
+
+ // Drawing these lines well can be complicated. LLVM generally can't modify a
+ // program to access memory that it didn't before, so using very narrow
access
+ // units can prevent the compiler from using optimal access patterns. For
+ // example, suppose a run of bit-fields occupies four bytes in a struct. If
we
+ // split that into four 1-byte access units, then a sequence of assignments
+ // that doesn't touch all four bytes may have to be emitted with multiple
+ // 8-bit stores instead of a single 32-bit store. On the other hand, if we
use
+ // very wide access units, we may find ourselves emitting accesses to
+ // bit-fields we didn't really need to touch, just because LLVM was unable to
+ // clean up after us.
+
+ // It is desirable to have access units be aligned powers of 2 no larger than
+ // a register. (On non-strict alignment ISAs, the alignment requirement can
be
+ // dropped.) A three byte access unit will be accessed using 2-byte and
1-byte
+ // accesses and bit manipulation. If no bitfield straddles across the two
+ // separate accesses, it is better to have separate 2-byte and 1-byte access
+ // units, as then LLVM will not generate unnecessary memory accesses, or bit
+ // manipulation. Similarly, on a strict-alignment architecture, it is better
+ // to keep access-units naturally aligned, to avoid similar bit
+ // manipulation synthesizing larger unaligned accesses.
+
+ // We do this in two phases, processing a sequential run of bitfield
+ // declarations.
+
+ // a) Bitfields that share parts of a single byte are, of necessity, placed
in
+ // the same access unit. That unit will encompass a consecutive
+ // run where adjacent bitfields share parts of a byte. (The first bitfield of
+ // such an access unit will start at the beginning of a byte.)
+
+ // b) Accumulate adjacent access units when the combined unit is naturally
+ // sized, no larger than a register, and on a strict alignment ISA,
+ // aligned. Note that this requires lookahead to one or more subsequent
access
+ // units. For instance, consider a 2-byte access-unit followed by 2 1-byte
+ // units. We can merge that into a 4-byte access-unit, but we would not want
+ // to merge a 2-byte followed by a single 1-byte (and no available tail
+ // padding).
+
+ // This accumulation is prevented when:
+ // *) it would cross a zero-width bitfield (ABI-dependent), or
+ // *) one of the candidate access units contains a volatile bitfield, or
+ // *) fine-grained bitfield access option is in effect.
+
+ CharUnits RegSize =
+ bitsToCharUnits(Context.getTargetInfo().getRegisterWidth());
+ unsigned CharBits = Context.getCharWidth();
+
+ RecordDecl::field_iterator Begin = FieldEnd;
+ CharUnits StartOffset;
+ uint64_t BitSize;
+ CharUnits BestEndOffset;
+ RecordDecl::field_iterator BestEnd =
[clang] [clang] Better bitfield access units (PR #65742)
@@ -439,82 +444,194 @@
CGRecordLowering::accumulateBitFields(RecordDecl::field_iterator Field,
Members.push_back(MemberInfo(bitsToCharUnits(StartBitOffset),
MemberInfo::Field, nullptr, *Field));
}
-return;
+return Field;
}
- // Check if OffsetInRecord (the size in bits of the current run) is better
- // as a single field run. When OffsetInRecord has legal integer width, and
- // its bitfield offset is naturally aligned, it is better to make the
- // bitfield a separate storage component so as it can be accessed directly
- // with lower cost.
- auto IsBetterAsSingleFieldRun = [&](uint64_t OffsetInRecord,
- uint64_t StartBitOffset) {
-if (!Types.getCodeGenOpts().FineGrainedBitfieldAccesses)
- return false;
-if (OffsetInRecord < 8 || !llvm::isPowerOf2_64(OffsetInRecord) ||
-!DataLayout.fitsInLegalInteger(OffsetInRecord))
- return false;
-// Make sure StartBitOffset is naturally aligned if it is treated as an
-// IType integer.
-if (StartBitOffset %
-Context.toBits(getAlignment(getIntNType(OffsetInRecord))) !=
-0)
- return false;
-return true;
- };
+ // The SysV ABI can overlap bitfield storage units with both other bitfield
+ // storage units /and/ other non-bitfield data members. Accessing a sequence
+ // of bitfields mustn't interfere with adjacent non-bitfields -- they're
+ // permitted to be accessed in separate threads for instance.
+
+ // We split runs of bit-fields into a sequence of "access units". When we
emit
+ // a load or store of a bit-field, we'll load/store the entire containing
+ // access unit. As mentioned, the standard requires that these loads and
+ // stores must not interfere with accesses to other memory locations, and it
+ // defines the bit-field's memory location as the current run of
+ // non-zero-width bit-fields. So an access unit must never overlap with
+ // non-bit-field storage or cross a zero-width bit-field. Otherwise, we're
+ // free to draw the lines as we see fit.
+
+ // Drawing these lines well can be complicated. LLVM generally can't modify a
+ // program to access memory that it didn't before, so using very narrow
access
+ // units can prevent the compiler from using optimal access patterns. For
+ // example, suppose a run of bit-fields occupies four bytes in a struct. If
we
+ // split that into four 1-byte access units, then a sequence of assignments
+ // that doesn't touch all four bytes may have to be emitted with multiple
+ // 8-bit stores instead of a single 32-bit store. On the other hand, if we
use
+ // very wide access units, we may find ourselves emitting accesses to
+ // bit-fields we didn't really need to touch, just because LLVM was unable to
+ // clean up after us.
+
+ // It is desirable to have access units be aligned powers of 2 no larger than
+ // a register. (On non-strict alignment ISAs, the alignment requirement can
be
+ // dropped.) A three byte access unit will be accessed using 2-byte and
1-byte
+ // accesses and bit manipulation. If no bitfield straddles across the two
+ // separate accesses, it is better to have separate 2-byte and 1-byte access
+ // units, as then LLVM will not generate unnecessary memory accesses, or bit
+ // manipulation. Similarly, on a strict-alignment architecture, it is better
+ // to keep access-units naturally aligned, to avoid similar bit
+ // manipulation synthesizing larger unaligned accesses.
+
+ // We do this in two phases, processing a sequential run of bitfield
+ // declarations.
+
+ // a) Bitfields that share parts of a single byte are, of necessity, placed
in
+ // the same access unit. That unit will encompass a consecutive
+ // run where adjacent bitfields share parts of a byte. (The first bitfield of
+ // such an access unit will start at the beginning of a byte.)
+
+ // b) Accumulate adjacent access units when the combined unit is naturally
+ // sized, no larger than a register, and on a strict alignment ISA,
+ // aligned. Note that this requires lookahead to one or more subsequent
access
+ // units. For instance, consider a 2-byte access-unit followed by 2 1-byte
+ // units. We can merge that into a 4-byte access-unit, but we would not want
+ // to merge a 2-byte followed by a single 1-byte (and no available tail
+ // padding).
+
+ // This accumulation is prevented when:
+ // *) it would cross a zero-width bitfield (ABI-dependent), or
+ // *) one of the candidate access units contains a volatile bitfield, or
+ // *) fine-grained bitfield access option is in effect.
+
+ CharUnits RegSize =
+ bitsToCharUnits(Context.getTargetInfo().getRegisterWidth());
+ unsigned CharBits = Context.getCharWidth();
+
+ RecordDecl::field_iterator Begin = FieldEnd;
+ CharUnits StartOffset;
+ uint64_t BitSize;
+ CharUnits BestEndOffset;
+ RecordDecl::field_iterator BestEnd =
[clang] [clang] Better bitfield access units (PR #65742)
urnathan wrote: @rjmccall here's a refactoring along the lines you suggested. Once one stops worrying about rescanning when the next access unit fails to accumulate into the current one, things get simpler. The compiler should be able to turn the boolean conditional flow within the loop body into direct control flow. Would you like the API change I introduced in the previous patch (having `accumulateBitFields` return the following non-bitfield) broken out into a separate PR? https://github.com/llvm/llvm-project/pull/65742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Better bitfield access units (PR #65742)
urnathan wrote: This update * moves the creation and installation of the BitFieldAccessUnit objects into CGRecordLowering. * adds aarch64 and arm darwin target tests, as those have different layout rules (let me know if I've missed something here). * Simplifies the checking of barriers -- an explicit flag, rather than inferring from a zero-sized unit. The latter can represent a non-barrier that happens to fall at a byte boundary, and results in a simplification of the gathering operation. * changes the API of accumulateBitFields to return the following non-bitfield. This means that the caller does not itself have to scan across the bitfields to find their range. Given the bitfield processor has scan the bitfields anyway, this seems like a win. * because of that, the determination of the limiting boundary, only needed for non-ms abis, is moved to where it is needed. FWIW, I experimented with the representation in BitFieldAccessUnit: * Although adjacent units are contiguous and it would seem one of Begin and End is unneeded, it turned out easiest to have both -- omitting Begin simply makes the calling code to the bookwork * Whether to represent access unit boundaries as bit or char offsets. Whichever is chosen leads to conversions to the other representation at various points. Using chars was the least painful, and makes it clear these are char boundaries. It might be possible to go further and perform the accumulation you outlined above, but there's enough churn in this update that I thought it best to push it. https://github.com/llvm/llvm-project/pull/65742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Better bitfield access units (PR #65742)
@@ -394,33 +412,155 @@ void CGRecordLowering::accumulateFields() {
: getStorageType(*Field),
*Field));
++Field;
-} else {
- ++Field;
}
}
}
-void
-CGRecordLowering::accumulateBitFields(RecordDecl::field_iterator Field,
- RecordDecl::field_iterator FieldEnd) {
- // Run stores the first element of the current run of bitfields. FieldEnd is
- // used as a special value to note that we don't have a current run. A
- // bitfield run is a contiguous collection of bitfields that can be stored in
- // the same storage block. Zero-sized bitfields and bitfields that would
- // cross an alignment boundary break a run and start a new one.
- RecordDecl::field_iterator Run = FieldEnd;
- // Tail is the offset of the first bit off the end of the current run. It's
- // used to determine if the ASTRecordLayout is treating these two bitfields
as
- // contiguous. StartBitOffset is offset of the beginning of the Run.
- uint64_t StartBitOffset, Tail = 0;
+namespace {
+
+// A run of bitfields assigned to the same access unit -- the size of memory
+// loads & stores.
+class BitFieldAccessUnit {
+ RecordDecl::field_iterator Begin; // Field at start of this access unit.
+ RecordDecl::field_iterator End; // Field just after this access unit.
+
+ CharUnits StartOffset; // Starting offset in the containing record.
+ CharUnits EndOffset; // Finish offset (exclusive) in the containing record.
+
+ bool ContainsVolatile; // This access unit contains a volatile bitfield.
+
+public:
+ // End barrier constructor.
+ BitFieldAccessUnit(RecordDecl::field_iterator F, CharUnits Offset,
+ bool Volatile = false)
+ : Begin(F), End(F), StartOffset(Offset), EndOffset(Offset),
+ContainsVolatile(Volatile) {}
+
+ // Collect contiguous bitfields into an access unit.
+ BitFieldAccessUnit(RecordDecl::field_iterator FieldBegin,
+ RecordDecl::field_iterator FieldEnd,
+ const CGRecordLowering );
+
+ // Compute the access unit following this one -- which might be a barrier at
+ // Limit.
+ BitFieldAccessUnit accumulateNextUnit(RecordDecl::field_iterator FieldEnd,
+CharUnits Limit,
+const CGRecordLowering ) const {
+return end() != FieldEnd ? BitFieldAccessUnit(end(), FieldEnd, CGRL)
+ : BitFieldAccessUnit(FieldEnd, Limit);
+ }
+ // Re-set the end of this unit if there is space before Probe starts.
+ void enlargeIfSpace(const BitFieldAccessUnit , CharUnits Offset) {
+if (Probe.getStartOffset() >= Offset) {
+ End = Probe.begin();
+ EndOffset = Offset;
+}
+ }
+
+public:
+ RecordDecl::field_iterator begin() const { return Begin; }
+ RecordDecl::field_iterator end() const { return End; }
+
+public:
+ // Accessors
+ CharUnits getSize() const { return EndOffset - StartOffset; }
+ CharUnits getStartOffset() const { return StartOffset; }
+ CharUnits getEndOffset() const { return EndOffset; }
+
+ // Predicates
+ bool isBarrier() const { return getSize().isZero(); }
+ bool hasVolatile() const { return ContainsVolatile; }
+
+ // Create the containing access unit and install the bitfields.
+ void installUnit(CGRecordLowering ) const {
+if (!isBarrier()) {
+ // Add the storage member for the access unit to the record. The
+ // bitfields get the offset of their storage but come afterward and
remain
+ // there after a stable sort.
+ llvm::Type *Type = CGRL.getIntNType(CGRL.Context.toBits(getSize()));
+ CGRL.Members.push_back(CGRL.StorageInfo(getStartOffset(), Type));
+ for (auto F : *this)
+if (!F->isZeroLengthBitField(CGRL.Context))
+ CGRL.Members.push_back(CGRecordLowering::MemberInfo(
+ getStartOffset(), CGRecordLowering::MemberInfo::Field, nullptr,
+ F));
+}
+ }
+};
+
+// Create an access unit of contiguous bitfields.
+BitFieldAccessUnit::BitFieldAccessUnit(RecordDecl::field_iterator FieldBegin,
+ RecordDecl::field_iterator FieldEnd,
+ const CGRecordLowering )
+: BitFieldAccessUnit(FieldBegin, CharUnits::Zero(),
+ FieldBegin->getType().isVolatileQualified()) {
+ assert(End != FieldEnd);
+
+ uint64_t StartBit = CGRL.getFieldBitOffset(*FieldBegin);
+ uint64_t BitSize = End->getBitWidthValue(CGRL.Context);
+ unsigned CharBits = CGRL.Context.getCharWidth();
+
+ assert(!(StartBit % CharBits) && "Not at start of char");
+
+ ++End;
+ if (BitSize ||
+ !(CGRL.Context.getTargetInfo().useZeroLengthBitfieldAlignment() ||
+CGRL.Context.getTargetInfo().useBitFieldTypeAlignment()))
+// The first field is not a (zero-width) barrier. Collect contiguous
fields.
+for (; End != FieldEnd; ++End) {
+ uint64_t BitOffset =
[clang] [clang] Better bitfield access units (PR #65742)
@@ -394,33 +412,155 @@ void CGRecordLowering::accumulateFields() {
: getStorageType(*Field),
*Field));
++Field;
-} else {
- ++Field;
}
}
}
-void
-CGRecordLowering::accumulateBitFields(RecordDecl::field_iterator Field,
- RecordDecl::field_iterator FieldEnd) {
- // Run stores the first element of the current run of bitfields. FieldEnd is
- // used as a special value to note that we don't have a current run. A
- // bitfield run is a contiguous collection of bitfields that can be stored in
- // the same storage block. Zero-sized bitfields and bitfields that would
- // cross an alignment boundary break a run and start a new one.
- RecordDecl::field_iterator Run = FieldEnd;
- // Tail is the offset of the first bit off the end of the current run. It's
- // used to determine if the ASTRecordLayout is treating these two bitfields
as
- // contiguous. StartBitOffset is offset of the beginning of the Run.
- uint64_t StartBitOffset, Tail = 0;
+namespace {
+
+// A run of bitfields assigned to the same access unit -- the size of memory
+// loads & stores.
+class BitFieldAccessUnit {
+ RecordDecl::field_iterator Begin; // Field at start of this access unit.
+ RecordDecl::field_iterator End; // Field just after this access unit.
+
+ CharUnits StartOffset; // Starting offset in the containing record.
+ CharUnits EndOffset; // Finish offset (exclusive) in the containing record.
+
+ bool ContainsVolatile; // This access unit contains a volatile bitfield.
+
+public:
+ // End barrier constructor.
+ BitFieldAccessUnit(RecordDecl::field_iterator F, CharUnits Offset,
+ bool Volatile = false)
+ : Begin(F), End(F), StartOffset(Offset), EndOffset(Offset),
+ContainsVolatile(Volatile) {}
+
+ // Collect contiguous bitfields into an access unit.
+ BitFieldAccessUnit(RecordDecl::field_iterator FieldBegin,
+ RecordDecl::field_iterator FieldEnd,
+ const CGRecordLowering );
urnathan wrote:
Updated to add gatherBitFieldAccessUnit and installBitFieldAccessUnit members
of CGRecordLowering. You're right that it makes the structure clearer.
https://github.com/llvm/llvm-project/pull/65742
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Better bitfield access units (PR #65742)
urnathan wrote: > > > > Thinking further, several (AArch64, ARM & Loongson) targets have a > 'HasUnaligned' bool in their TargetInfo objects. Perhaps the way forwards is > to promote that to the base TargetInfo and just use that, rather than the > bitfield-specific field I added (which just duplicates that functionality on > those targets)? I've added such a change. The AArch64 & ARM targets record this information to determine whether a `__ARM_FEATURE_UNALIGNED` macro should be defined. The Loongson target just needs to check the `-ual` target feature. https://github.com/llvm/llvm-project/pull/65742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Better bitfield access units (PR #65742)
urnathan wrote: > Yeah, LLVM supports changing subtarget options on a per-function basis. We > would presumably make the query based on the global setting. > > Anyway, getting this information from LLVM doesn't seem tractable in the > short-to-medium term; it's just unfortunate. Thinking further, several (AArch64, ARM & Loongson) targets have a 'HasUnaligned' bool in their TargetInfo objects. Perhaps the way forwards is to promote that to the base TargetInfo and just use that, rather than the bitfield-specific field I added (which just duplicates that functionality on those targets)? https://github.com/llvm/llvm-project/pull/65742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Better bitfield access units (PR #65742)
@@ -132,6 +132,7 @@ class LLVM_LIBRARY_VISIBILITY LoongArch64TargetInfo
: LoongArchTargetInfo(Triple, Opts) {
LongWidth = LongAlign = PointerWidth = PointerAlign = 64;
IntMaxType = Int64Type = SignedLong;
+HasCheapUnalignedBitfieldAccess = true;
urnathan wrote:
fixed
https://github.com/llvm/llvm-project/pull/65742
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] constify or staticify some CGRecordLowering fns (PR #82874)
https://github.com/urnathan closed https://github.com/llvm/llvm-project/pull/82874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] constify or staticify some CGRecordLowering fns (PR #82874)
https://github.com/urnathan updated
https://github.com/llvm/llvm-project/pull/82874
>From 792c608cc55d89552cf86d058796825a1f428f5d Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Fri, 2 Feb 2024 08:01:21 -0500
Subject: [PATCH] [clang][NFC] constify or staticify some fns
These functions do not alter the object -- and on one case don't even
need it. Thus marking them static or const as appropriate.
---
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 28 ++---
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 868ef810f3c4e8..7822903b89ce47 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -95,7 +95,7 @@ struct CGRecordLowering {
CGRecordLowering(CodeGenTypes , const RecordDecl *D, bool Packed);
// Short helper routines.
/// Constructs a MemberInfo instance from an offset and llvm::Type *.
- MemberInfo StorageInfo(CharUnits Offset, llvm::Type *Data) {
+ static MemberInfo StorageInfo(CharUnits Offset, llvm::Type *Data) {
return MemberInfo(Offset, MemberInfo::Field, Data);
}
@@ -104,7 +104,7 @@ struct CGRecordLowering {
/// fields of the same formal type. We want to emit a layout with
/// these discrete storage units instead of combining them into a
/// continuous run.
- bool isDiscreteBitFieldABI() {
+ bool isDiscreteBitFieldABI() const {
return Context.getTargetInfo().getCXXABI().isMicrosoft() ||
D->isMsStruct(Context);
}
@@ -121,22 +121,22 @@ struct CGRecordLowering {
/// other bases, which complicates layout in specific ways.
///
/// Note specifically that the ms_struct attribute doesn't change this.
- bool isOverlappingVBaseABI() {
+ bool isOverlappingVBaseABI() const {
return !Context.getTargetInfo().getCXXABI().isMicrosoft();
}
/// Wraps llvm::Type::getIntNTy with some implicit arguments.
- llvm::Type *getIntNType(uint64_t NumBits) {
+ llvm::Type *getIntNType(uint64_t NumBits) const {
unsigned AlignedBits = llvm::alignTo(NumBits, Context.getCharWidth());
return llvm::Type::getIntNTy(Types.getLLVMContext(), AlignedBits);
}
/// Get the LLVM type sized as one character unit.
- llvm::Type *getCharType() {
+ llvm::Type *getCharType() const {
return llvm::Type::getIntNTy(Types.getLLVMContext(),
Context.getCharWidth());
}
/// Gets an llvm type of size NumChars and alignment 1.
- llvm::Type *getByteArrayType(CharUnits NumChars) {
+ llvm::Type *getByteArrayType(CharUnits NumChars) const {
assert(!NumChars.isZero() && "Empty byte arrays aren't allowed.");
llvm::Type *Type = getCharType();
return NumChars == CharUnits::One() ? Type :
@@ -144,7 +144,7 @@ struct CGRecordLowering {
}
/// Gets the storage type for a field decl and handles storage
/// for itanium bitfields that are smaller than their declared type.
- llvm::Type *getStorageType(const FieldDecl *FD) {
+ llvm::Type *getStorageType(const FieldDecl *FD) const {
llvm::Type *Type = Types.ConvertTypeForMem(FD->getType());
if (!FD->isBitField()) return Type;
if (isDiscreteBitFieldABI()) return Type;
@@ -152,29 +152,29 @@ struct CGRecordLowering {
(unsigned)Context.toBits(getSize(Type;
}
/// Gets the llvm Basesubobject type from a CXXRecordDecl.
- llvm::Type *getStorageType(const CXXRecordDecl *RD) {
+ llvm::Type *getStorageType(const CXXRecordDecl *RD) const {
return Types.getCGRecordLayout(RD).getBaseSubobjectLLVMType();
}
- CharUnits bitsToCharUnits(uint64_t BitOffset) {
+ CharUnits bitsToCharUnits(uint64_t BitOffset) const {
return Context.toCharUnitsFromBits(BitOffset);
}
- CharUnits getSize(llvm::Type *Type) {
+ CharUnits getSize(llvm::Type *Type) const {
return CharUnits::fromQuantity(DataLayout.getTypeAllocSize(Type));
}
- CharUnits getAlignment(llvm::Type *Type) {
+ CharUnits getAlignment(llvm::Type *Type) const {
return CharUnits::fromQuantity(DataLayout.getABITypeAlign(Type));
}
- bool isZeroInitializable(const FieldDecl *FD) {
+ bool isZeroInitializable(const FieldDecl *FD) const {
return Types.isZeroInitializable(FD->getType());
}
- bool isZeroInitializable(const RecordDecl *RD) {
+ bool isZeroInitializable(const RecordDecl *RD) const {
return Types.isZeroInitializable(RD);
}
void appendPaddingBytes(CharUnits Size) {
if (!Size.isZero())
FieldTypes.push_back(getByteArrayType(Size));
}
- uint64_t getFieldBitOffset(const FieldDecl *FD) {
+ uint64_t getFieldBitOffset(const FieldDecl *FD) const {
return Layout.getFieldOffset(FD->getFieldIndex());
}
// Layout routines.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 8e22fff - [clang] Remove trailing whitespace
Author: Nathan Sidwell Date: 2024-02-24T12:25:08-05:00 New Revision: 8e22fffc85b36784146041499b716cec74285660 URL: https://github.com/llvm/llvm-project/commit/8e22fffc85b36784146041499b716cec74285660 DIFF: https://github.com/llvm/llvm-project/commit/8e22fffc85b36784146041499b716cec74285660.diff LOG: [clang] Remove trailing whitespace Fix commit 66f6929fec3ae Added: Modified: clang/docs/HLSL/ExpectedDifferences.rst Removed: diff --git a/clang/docs/HLSL/ExpectedDifferences.rst b/clang/docs/HLSL/ExpectedDifferences.rst index 60001b22dc7920..d1b6010f10f43a 100644 --- a/clang/docs/HLSL/ExpectedDifferences.rst +++ b/clang/docs/HLSL/ExpectedDifferences.rst @@ -93,7 +93,7 @@ behavior between Clang and DXC. Some examples include: fma(X, Y, Z); // DXC: Fails to resolve no known conversion from float to double. // Clang: Resolves to fma(double,double,double). #endif - + double D = dot(A, B); // DXC: Resolves to dot(double3, double3), fails DXIL Validation. // FXC: Expands to compute double dot product with fmul/fadd // Clang: Resolves to dot(float3, float3), emits conversion warnings. @@ -102,7 +102,7 @@ behavior between Clang and DXC. Some examples include: .. note:: - In Clang, a conscious decision was made to exclude the ``dot(vector, vector)`` + In Clang, a conscious decision was made to exclude the ``dot(vector, vector)`` overload and allow overload resolution to resolve the ``vector`` overload. This approach provides ``-Wconversion`` diagnostic notifying the user of the conversion rather than silently altering ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] constify or staticify some CGRecordLowering fns (PR #82874)
https://github.com/urnathan edited https://github.com/llvm/llvm-project/pull/82874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] constify or staticify some fns (PR #82874)
https://github.com/urnathan created
https://github.com/llvm/llvm-project/pull/82874
As mentioned in PR https://github.com/llvm/llvm-project/pull/65742, these
functions do not alter the object -- and on one case don't even need it. Thus
marking them static or const as appropriate. I constified a few more than I'd
originally fallen over.
>From ccc917e8a826bbefcd68df4e013edbd97bb19914 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Fri, 2 Feb 2024 08:01:21 -0500
Subject: [PATCH] [clang][NFC] constify or staticify some fns
These functions do not alter the object -- and on one case don't even
need it. Thus marking them static or const as appropriate.
---
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 28 ++---
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 868ef810f3c4e8..7822903b89ce47 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -95,7 +95,7 @@ struct CGRecordLowering {
CGRecordLowering(CodeGenTypes , const RecordDecl *D, bool Packed);
// Short helper routines.
/// Constructs a MemberInfo instance from an offset and llvm::Type *.
- MemberInfo StorageInfo(CharUnits Offset, llvm::Type *Data) {
+ static MemberInfo StorageInfo(CharUnits Offset, llvm::Type *Data) {
return MemberInfo(Offset, MemberInfo::Field, Data);
}
@@ -104,7 +104,7 @@ struct CGRecordLowering {
/// fields of the same formal type. We want to emit a layout with
/// these discrete storage units instead of combining them into a
/// continuous run.
- bool isDiscreteBitFieldABI() {
+ bool isDiscreteBitFieldABI() const {
return Context.getTargetInfo().getCXXABI().isMicrosoft() ||
D->isMsStruct(Context);
}
@@ -121,22 +121,22 @@ struct CGRecordLowering {
/// other bases, which complicates layout in specific ways.
///
/// Note specifically that the ms_struct attribute doesn't change this.
- bool isOverlappingVBaseABI() {
+ bool isOverlappingVBaseABI() const {
return !Context.getTargetInfo().getCXXABI().isMicrosoft();
}
/// Wraps llvm::Type::getIntNTy with some implicit arguments.
- llvm::Type *getIntNType(uint64_t NumBits) {
+ llvm::Type *getIntNType(uint64_t NumBits) const {
unsigned AlignedBits = llvm::alignTo(NumBits, Context.getCharWidth());
return llvm::Type::getIntNTy(Types.getLLVMContext(), AlignedBits);
}
/// Get the LLVM type sized as one character unit.
- llvm::Type *getCharType() {
+ llvm::Type *getCharType() const {
return llvm::Type::getIntNTy(Types.getLLVMContext(),
Context.getCharWidth());
}
/// Gets an llvm type of size NumChars and alignment 1.
- llvm::Type *getByteArrayType(CharUnits NumChars) {
+ llvm::Type *getByteArrayType(CharUnits NumChars) const {
assert(!NumChars.isZero() && "Empty byte arrays aren't allowed.");
llvm::Type *Type = getCharType();
return NumChars == CharUnits::One() ? Type :
@@ -144,7 +144,7 @@ struct CGRecordLowering {
}
/// Gets the storage type for a field decl and handles storage
/// for itanium bitfields that are smaller than their declared type.
- llvm::Type *getStorageType(const FieldDecl *FD) {
+ llvm::Type *getStorageType(const FieldDecl *FD) const {
llvm::Type *Type = Types.ConvertTypeForMem(FD->getType());
if (!FD->isBitField()) return Type;
if (isDiscreteBitFieldABI()) return Type;
@@ -152,29 +152,29 @@ struct CGRecordLowering {
(unsigned)Context.toBits(getSize(Type;
}
/// Gets the llvm Basesubobject type from a CXXRecordDecl.
- llvm::Type *getStorageType(const CXXRecordDecl *RD) {
+ llvm::Type *getStorageType(const CXXRecordDecl *RD) const {
return Types.getCGRecordLayout(RD).getBaseSubobjectLLVMType();
}
- CharUnits bitsToCharUnits(uint64_t BitOffset) {
+ CharUnits bitsToCharUnits(uint64_t BitOffset) const {
return Context.toCharUnitsFromBits(BitOffset);
}
- CharUnits getSize(llvm::Type *Type) {
+ CharUnits getSize(llvm::Type *Type) const {
return CharUnits::fromQuantity(DataLayout.getTypeAllocSize(Type));
}
- CharUnits getAlignment(llvm::Type *Type) {
+ CharUnits getAlignment(llvm::Type *Type) const {
return CharUnits::fromQuantity(DataLayout.getABITypeAlign(Type));
}
- bool isZeroInitializable(const FieldDecl *FD) {
+ bool isZeroInitializable(const FieldDecl *FD) const {
return Types.isZeroInitializable(FD->getType());
}
- bool isZeroInitializable(const RecordDecl *RD) {
+ bool isZeroInitializable(const RecordDecl *RD) const {
return Types.isZeroInitializable(RD);
}
void appendPaddingBytes(CharUnits Size) {
if (!Size.isZero())
FieldTypes.push_back(getByteArrayType(Size));
}
- uint64_t getFieldBitOffset(const FieldDecl *FD) {
+ uint64_t getFieldBitOffset(const FieldDecl *FD) const {
return
[clang] [clang][NFC] Adjust TBAA Base Info API (PR #73263)
@@ -95,8 +95,6 @@ static bool TypeHasMayAlias(QualType QTy) {
/// Check if the given type is a valid base type to be used in access tags.
static bool isValidBaseType(QualType QTy) {
- if (QTy->isReferenceType())
-return false;
urnathan wrote:
would you prefer I separate this piece out to a separate PR?
https://github.com/llvm/llvm-project/pull/73263
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Better bitfield access units (PR #65742)
@@ -132,6 +132,7 @@ class LLVM_LIBRARY_VISIBILITY LoongArch64TargetInfo
: LoongArchTargetInfo(Triple, Opts) {
LongWidth = LongAlign = PointerWidth = PointerAlign = 64;
IntMaxType = Int64Type = SignedLong;
+HasCheapUnalignedBitfieldAccess = true;
urnathan wrote:
you say set to true when -mno-unaligned-access, but I think you mean set to
false in that case?
https://github.com/llvm/llvm-project/pull/65742
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Better bitfield access units (PR #65742)
urnathan wrote: > On the target hook, it's a shame we can't easily get this information from > LLVM. I believe it's already there — `TargetLowering` has an > `allowsMisalignedMemoryAccesses` method that includes some approximation of > how fast a particular access would be. In practice, it seems to be quite > complex and often specific to the type and subtarget. Maybe it'd be worth > starting a conversation with LLVM folks to see if this could reasonably be > lifted somewhere we could use it? Agreed, I did look at that, and AFAICT the allowsMisalignedMemoryAccess is a property of the llvm subtarget, which can, IIUC, change on a per-function basis? One at least needs a function codegen context around? And, clang needs to generate the llvm-structure in non-function contexts (eg namespace-scope variables). I'd really like to not tangle this particular change with a complex reorg. I had a discourse thread at, https://discourse.llvm.org/t/targetinfo-for-unaligned-load-capability/72832 about such a target hook. https://github.com/llvm/llvm-project/pull/65742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Strict aliasing warning ala GCC [PR50066] (PR #74155)
urnathan wrote: > On the LLVM side, there's very little interesting logic; it's basically just > walking the tree of metadata nodes generated by clang. See > https://llvm.org/docs/LangRef.html#tbaa-node-semantics . The hard part of the > refactoring would just be adding an abstraction for the underlying > information. IIUC you're suggesting movint the TBAA data origination from CodeGen into (probably) Sema and/or TargetInfo and then deriving the LLVM info in CodeGen from that. I.e. keep once source of truth, but move where it is? I don't think it'd be that hard -- mostly mechanical. And I suspect only the scalar TBAA needs so moving, the structure-related stuff is sufficiently different to that strict-aliasing wants. Although you don't explicitly say it, the implication is you'd be ameanable to such an approach? https://github.com/llvm/llvm-project/pull/74155 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Better bitfield access units (PR #65742)
urnathan wrote:
@rjmccall thanks for your comments. Here's a reimplementation using a single
loop -- the downside is that we may end up repeating the initial grouping scan,
if we search more than 1 Access Unit ahead and fail to find a 'good' access
unit. This update changes the algorithm slightly, as I realized that `struct A
{ short a: 16; char b:8; int c;};` and `struct B { char b: 8; short a:16; int
c;};` should have the same (32-bit) access unit. Originally only `A` got that.
1) I noticed that several `CGRecordLowering` member fns could either be `const`
or `static` -- would you like that as a separate NFC PR?
2) I have corrected the error about merging across zero-sized members.
3) It may not be obvious from GH, but this PR breaks down to 3 (or 4) separate
commits:
a) A set of test cases, marked up for the current scheme
b) A new Target hook indicating whether unaligned accesses are cheap
c) [optional] the CGRecordLowering change just mentioned
d) the algorithm change, which updates those tests and makes it easier to
see how the behaviour changes.
Do you want that commit structure maintained?
https://github.com/llvm/llvm-project/pull/65742
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Better bitfield access units (PR #65742)
@@ -376,33 +377,41 @@ void CGRecordLowering::lowerUnion(bool isNoUniqueAddress)
{
}
void CGRecordLowering::accumulateFields() {
- for (RecordDecl::field_iterator Field = D->field_begin(),
- FieldEnd = D->field_end();
-Field != FieldEnd;) {
+ RecordDecl::field_iterator FieldEnd = D->field_end();
+ RecordDecl::field_iterator BitField = FieldEnd;
+ for (RecordDecl::field_iterator Field = D->field_begin(); Field != FieldEnd;
+ ++Field) {
if (Field->isBitField()) {
- RecordDecl::field_iterator Start = Field;
- // Iterate to gather the list of bitfields.
- for (++Field; Field != FieldEnd && Field->isBitField(); ++Field);
- accumulateBitFields(Start, Field);
+ if (BitField == FieldEnd)
+// Start gathering bitfields
+BitField = Field;
} else if (!Field->isZeroSize(Context)) {
// Use base subobject layout for the potentially-overlapping field,
// as it is done in RecordLayoutBuilder
+ CharUnits Offset = bitsToCharUnits(getFieldBitOffset(*Field));
urnathan wrote:
ah, thanks -- I should have checked harder. Simple enough to undo that bit.
https://github.com/llvm/llvm-project/pull/65742
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Adjust TBAA Base Info API (PR #73263)
@@ -95,8 +95,6 @@ static bool TypeHasMayAlias(QualType QTy) {
/// Check if the given type is a valid base type to be used in access tags.
static bool isValidBaseType(QualType QTy) {
- if (QTy->isReferenceType())
-return false;
urnathan wrote:
This is case #1 in the intro. Namely this is immediately followed by
```
if (const RecordType *TTy = QTy->getAs()) {
.. do stuff
return true;
}
return false;
```
So AFAICT that inital check for a reference type is doing nothing. Am I
misunderstanding getAs and that it's possible for a type to be both
RecordType and ReferenceType?
https://github.com/llvm/llvm-project/pull/73263
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Better bitfield access units (PR #65742)
urnathan wrote: Can someone please review this? https://github.com/llvm/llvm-project/pull/65742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Adjust TBAA Base Info API (PR #73263)
urnathan wrote: @efriedma-quic this naming ok? https://github.com/llvm/llvm-project/pull/73263 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Better bitfield access units (PR #65742)
urnathan wrote: ping? https://github.com/llvm/llvm-project/pull/65742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Adjust TBAA Base Info API (PR #73263)
urnathan wrote: ping? https://github.com/llvm/llvm-project/pull/73263 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Strict aliasing warning ala GCC [PR50066] (PR #74155)
urnathan wrote: > > > FWIW the GCC doc is > > > https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wstrict-aliasing_003dn > > > It says for Level 3 "If optimization is enabled, it also runs in the > > > back end, where it deals with multiple statement cases using > > > flow-sensitive points-to information." > > > Do you know how it works? Any example? > > > > > > I do not now how it works -- didn't go poking there. Neither do I have > > examples. > > My understanding (which could be totally wrong) is that the logic for when to > emit the level 3 diagnostics rests mostly in the optimizer passes. This is > not something I think Clang should emulate as it creates a very poor user > experience in practice (not just with strict aliasing diagnostics -- I don't > think _any_ diagnostics other than remarks should be emitted based on LLVM > optimization decisions aside from the `error` and `warning` attributes which > are special circumstances). I don't know if it's 'mostly', the level 3 can certainly be triggered in the FE -- it requires (a) an address-of-object idiom, (b) a suspicious cast and (c) a dereference. Something like `*reinterpret_cast() = 5`, where `obj` is not compatible with `int`. That's what this patch implements. (In GCC's case when it warns about this in the FE it marks the tree as warned, to inhibit the middle end also doing so.) https://github.com/llvm/llvm-project/pull/74155 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Strict aliasing warning ala GCC [PR50066] (PR #74155)
@@ -498,3 +498,137 @@
CodeGenTBAA::mergeTBAAInfoForMemoryTransfer(TBAAAccessInfo DestInfo,
// access type regardless of their base types.
return TBAAAccessInfo::getMayAliasInfo();
}
+
+// Determine the aliasing kind bit-converting from type Src to type Dst.
+CodeGenTBAA::AliasingKind CodeGenTBAA::getAliasingKind(QualType ,
+ QualType ) {
+ assert(!Src->isVoidType() && !Dst->isVoidType());
+ if (TypeHasMayAlias(Src) || TypeHasMayAlias(Dst))
+return AK_Ok;
+
+ Src = QualType{Src->getBaseElementTypeUnsafe(), 0};
+ Dst = QualType{Dst->getBaseElementTypeUnsafe(), 0};
+
+ auto *SrcDecl = Src->getAsRecordDecl();
+ auto *DstDecl = Dst->getAsRecordDecl();
+
+ const llvm::MDNode *AnyTBAA = getChar();
+ const llvm::MDNode *SrcTBAA = nullptr;
+ const llvm::MDNode *DstTBAA = nullptr;
+
+ if (!SrcDecl) {
+SrcTBAA = getTypeInfo(Src);
+if (!SrcTBAA || SrcTBAA == AnyTBAA)
urnathan wrote:
Pedantically 'signed char' is not blessed with that property by the std. But
many compilers (llvm included) bestow it upon signed char. Indeed this
particular patch doesn't make a judgement on that, it just gets the aliasing
info for 'signed char' and it happens to get the same ubiquitous char tbaa.
https://github.com/llvm/llvm-project/pull/74155
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Strict aliasing warning ala GCC [PR50066] (PR #74155)
@@ -37,6 +38,27 @@ class ASTConsumer {
friend class SemaConsumer;
+public:
+ /// Allow type-based aliasing information to be interrogated by the AST
+ /// producer (for diagnostics).
+ class TypeAliasing {
urnathan wrote:
Oh, we also endup deriving from the TypeAliasing class to override the virtual
function it provides.
https://github.com/llvm/llvm-project/pull/74155
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Adjust TBAA Base Info API (PR #73263)
urnathan wrote: > I'd tend to prefer to keep the simpler name for the external interface, and > use a more complicated one for use within the class. So maybe introduce > getValidBaseTypeInfo(). This naming scheme better? https://github.com/llvm/llvm-project/pull/73263 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Better bitfield access units (PR #65742)
urnathan wrote: Rebased https://github.com/llvm/llvm-project/pull/65742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Adjust TargetInfo bitfield (PR #74893)
https://github.com/urnathan closed https://github.com/llvm/llvm-project/pull/74893 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Adjust TargetInfo bitfield (PR #74893)
https://github.com/urnathan ready_for_review https://github.com/llvm/llvm-project/pull/74893 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Adjust TargetInfo bitfield (PR #74893)
https://github.com/urnathan created https://github.com/llvm/llvm-project/pull/74893 An 8 bit bitfield with preferred bool type? Seems confused. >From f7cac332123f6ea6c78dcffbcac2d58f356b6396 Mon Sep 17 00:00:00 2001 From: Nathan Sidwell Date: Fri, 8 Dec 2023 17:43:22 -0500 Subject: [PATCH] [clang] Adjust TargetInfo bitfield An 8 bit bitfield with preferred bool type? Seems confused. --- clang/include/clang/Basic/TargetInfo.h | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h index 41f3c2e403cbe..ec0189627dfbd 100644 --- a/clang/include/clang/Basic/TargetInfo.h +++ b/clang/include/clang/Basic/TargetInfo.h @@ -266,7 +266,6 @@ class TargetInfo : public TransferrableTargetInfo, LLVM_PREFERRED_TYPE(bool) unsigned AllowAMDGPUUnsafeFPAtomics : 1; - LLVM_PREFERRED_TYPE(bool) unsigned ARMCDECoprocMask : 8; unsigned MaxOpenCLWorkGroupSize; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Better bitfield access units (PR #65742)
urnathan wrote: > I'm planning to take a closer look at this when I have more time. Sorry I > haven't been more responsive here. When might that be? https://github.com/llvm/llvm-project/pull/65742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Strict aliasing warning ala GCC [PR50066] (PR #74155)
@@ -0,0 +1,192 @@
+// RUN: %clang_cc1 %s -O0 -Wstrict-aliasing -S -o %t -verify=quiet
+// RUN: %clang_cc1 %s -O2 -Wstrict-aliasing=0 -S -o %t -verify=quiet
+// RUN: %clang_cc1 %s -O2 -Wno-strict-aliasing -S -o %t -verify=quiet
+// RUN: %clang_cc1 %s -O2 -Wstrict-aliasing=1 -S -o %t
-verify=level1,level12,level123
+// RUN: %clang_cc1 %s -O2 -Wstrict-aliasing=2 -S -o %t
-verify=level2,level23,level12,level123
+// RUN: %clang_cc1 %s -O2 -Wstrict-aliasing=3 -S -o %t -verify=level23,level123
+// RUN: %clang_cc1 %s -O2 -Wstrict-aliasing -S -o %t -verify=level23,level123
+// RUN: %clang_cc1 %s -O2 -S -o %t -verify=level23,level123
+
+// quiet-no-diagnostics
+
+#if _LP64
+// These names make more sense on an ilp32 machine
+typedef long INT;
+typedef long long LONG;
+typedef unsigned long UINT;
+#else
+typedef int INT;
+typedef long LONG;
+typedef unsigned int UINT;
+#endif
+typedef short SHORT;
+
+INT ScalarINT;
+INT Ary[2];
+struct {int m;} Struct;
+
+_Complex int CPLx;
+
+void ByVal(long long);
+void ByPtr(void *);
+
+void VarPtr(INT *Ptr) {
+ // GCC: 1
+ // level1-warning@+1{{type-punned pointer might break}}
+ ByPtr((LONG *)(Ptr));
+ // level1-note@-1{{not alias compatible}}
+
+ // GCC:
+ ByPtr((LONG *)((void *)(Ptr)));
+
+ // GCC: 1
+ // level1-warning@+1{{type-punned pointer might break}}
+ ByVal(*(LONG *)(Ptr));
+ // level1-note@-1{{not alias compatible}}
+}
+
+void Object() {
+ // GCC: 1, 2
+ // level2-warning@+2{{type-punned pointer breaks}}
+ // level1-warning@+1{{type-punned pointer might break}}
+ ByPtr((LONG *)());
+ // level12-note@-1{{not alias compatible}}
+
+ // GCC:
+ ByPtr((LONG *)((void *)()));
+
+ // GCC: 1, 2, 3
+ // level23-warning@+2{{type-punned pointer breaks}}
+ // level1-warning@+1{{type-punned pointer might break}}
+ ByVal(*(LONG *)());
+ // level123-note@-1{{not alias compatible}}
+}
+
+// Level 1, 2, 3 - 1, 2, 3
+void DetectedVariants() {
+ // GCC: 1, 2, 3
+ // level23-warning@+2{{type-punned pointer breaks}}
+ // level1-warning@+1{{type-punned pointer might break}}
+ ByVal(*(LONG *)([1]));
+ // level123-note@-1{{not alias compatible}}
+
+ // GCC: 1, 2, 3
+ // level23-warning@+2{{type-punned pointer breaks}}
+ // level1-warning@+1{{type-punned pointer might break}}
+ ByVal(*(LONG *)());
+ // level123-note@-1{{not alias compatible}}
+
+ // GCC: 1, 2, 3
+ // level23-warning@+2{{type-punned pointer breaks}}
+ // level1-warning@+1{{type-punned pointer might break}}
+ ByVal(*(LONG *)(&()->m));
+ // level123-note@-1{{not alias compatible}}
+
+ // GCC: 1, 2, 3
+ // level23-warning@+2{{type-punned pointer breaks}}
+ // level1-warning@+1{{type-punned pointer might break}}
+ ByVal(*(LONG *)(&__real__(CPLx)));
+ // level123-note@-1{{not alias compatible}}
+
+ // GCC: 1, 2, 3
+ // level23-warning@+2{{type-punned pointer breaks}}
+ // level1-warning@+1{{type-punned pointer might break}}
+ ByVal(*(LONG *)(&__imag__(CPLx)));
+ // level123-note@-1{{not alias compatible}}
+}
+
+void Ok() {
+ // GCC:
+ ByPtr((UINT *)());
+ // GCC:
+ ByPtr((UINT *)((void *)()));
+ // GCC:
+ ByVal(*(UINT *)());
+}
+
+// Level 1, 2, 3 - 1, 2, 3
+void Parens() {
+ // GCC: 1, 2, 3
+ // level23-warning@+2{{type-punned pointer breaks}}
+ // level1-warning@+1{{type-punned pointer might break}}
+ ByVal(*((LONG *)((&(ScalarINT);
+ // level123-note@-1{{not alias compatible}}
+
+ // GCC: 1, 2, 3
+ // level23-warning@+2{{type-punned pointer breaks}}
+ // level1-warning@+1{{type-punned pointer might break}}
+ ByVal(*((LONG *)((&(Ary[1]);
+ // level123-note@-1{{not alias compatible}}
+}
+
+// Clang models may_alias as a decl attribute, not a type attribute.
+
+typedef int MA __attribute__((may_alias));
+
+void Frob(MA *a) {
+ ByPtr((short *)(a));
+ ByVal(*(short *)(a));
+}
+
+struct Inner { int m; };
+struct Outer1 { struct Inner i; };
+struct Outer2 { struct Outer1 o; };
+struct Inner i;
+struct Outer2 o;
+
+void ByValInner (struct Inner);
+void ByValOuter2 (struct Outer2);
+
+void Inherit() {
+ // Check we see through multiple levels
+ int in;
+
+ ByValOuter2(*(struct Outer2 *));
+ ByValOuter2(*(struct Outer2 *));
+ ByValInner(*(struct Inner *));
+ ByValInner(*(struct Inner *));
+ ByVal(*(int *));
+}
+
+// PR 50066
+typedef unsigned char uchar;
+
+void Double(double);
+
+int main() {
+ double d = 2.34;
+ int i[2];
+ Double(d);
+
+ // GCC: 1, 2, 3
+ // level23-warning@+2{{type-punned pointer breaks}}
+ // level1-warning@+1{{type-punned pointer might break}}
+ *(long long *)i =
+ // level123-note@-1{{not alias compatible}}
+
+ // GCC: 1, 2, 3
+ // level23-warning@+2{{type-punned pointer breaks}}
+ // level1-warning@+1{{type-punned pointer might break}}
+ *(long long *)
+ // level123-note@-1{{not alias compatible}}
+
+ // GCC: 1, 2, 3
+ // level23-warning@+2{{type-punned pointer breaks}}
+ // level1-warning@+1{{type-punned pointer might break}}
+ ((int *))[0] = i[0];
+ // level123-note@-1{{not
[clang] [clang] Strict aliasing warning ala GCC [PR50066] (PR #74155)
https://github.com/urnathan updated
https://github.com/llvm/llvm-project/pull/74155
>From 89c05618bb75fd073343046f3b54bde5f2254719 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Wed, 15 Nov 2023 15:27:16 -0500
Subject: [PATCH 1/6] [clang] Strict aliasing warning ala GCC [PR50066]
This implements -Wstrict-aliasing(=[123])? along the same lines as
GCC. It's not 100% the same for reasons expanded on below. The default
is level 3, and I have verified that bootstrapping does not trigger
any warnings (just like building with GCC).
As with GCC, higher levels result in fewer warnings, reducing the
number of false positives at the cost of missing (some) potential
cases. Unlike GCC, this is entirely in the FE, we do not propagate any
checking into the IR (so there are cases GCC will detect we do not, I
have not encountered any). GCC's documentation is not very specific
about which cases are detected. I examined GCC's source code to reverse
engineer the algorithm[*], and as can be seen from the testcases, noted
GCC's behaviour.
The strict aliasing check relies on TBAA. LLVM's representation is
semantically different to GCCs for structured types. I have tried to
keep with the spirit of the warning.
The warning checks reinterpret_casts that are CK_BitCast or
CK_LValueBitCast. It also checks C-style casts that are equivalent (to
the extent available, as a C-style bitcast could be a well-formed
static cast).
level=1 looks for reinterpret casts from T * to U * (or an lvalue of
type T to U &), where T and U are not TBAA compatible.
level=2 requires the src expression be an lvalue something of
known(ish) static type. I.e a variable, array dereference or member
access.
level=3 requires level 2 and that the resultant pointer is actually
referenced (lvalue->rvalue or lvalue written). Here we can do better
than GCC, which doesn't represent this in the IR -- we merely get a
dereference (and reference types are modeled as pointers with
auto-dereference semantics). There is one exception to this, which is
by-value aggregate arguments. These end up invoking a copy constructor
(passing a reference of course), but are IMHO morally an rvalue -- so
should trigger.
The warning hooks into clang's code-generator's TBAA machinery. For
scalar types the TBAA is straight forwards, comparing LLVM's MDNode
representaion. For record & union types we check if one of the types
is (recursively) the same (or TBAA compatible) as the first direct
base or a field(s) of the record at offset zero (i.e. the first member
of a record, or any members of a union). This covers both C++ and
C-Style inheritance. Also. the member maybe alias_any, or in
ubiquitous-char's alias set, which is also permissible.
The warning is similar to the alignment check that
CheckCompatibleReinterpretCast does, perhaps some merging could occur
if this is accepted?
[*] I implemented what turned into GCC's level=1 way back when.
WIP: adjust tests
---
clang/include/clang/AST/ASTConsumer.h | 25 +
clang/include/clang/Basic/DiagnosticGroups.td |6 -
clang/include/clang/Basic/DiagnosticOptions.h |4 +
.../clang/Basic/DiagnosticSemaKinds.td|8 +
clang/include/clang/Driver/Options.td |6 +
clang/include/clang/Sema/Sema.h | 11 +
clang/lib/CodeGen/BackendConsumer.h |1 +
clang/lib/CodeGen/CodeGenAction.cpp |4 +
clang/lib/CodeGen/CodeGenModule.h |1 +
clang/lib/CodeGen/CodeGenTBAA.cpp | 134 ++
clang/lib/CodeGen/CodeGenTBAA.h |5 +-
clang/lib/CodeGen/ModuleBuilder.cpp |2 +
clang/lib/Frontend/CompilerInvocation.cpp |3 +
clang/lib/Frontend/FrontendAction.cpp |8 +
clang/lib/Sema/SemaCast.cpp | 139 ++
clang/lib/Sema/SemaExpr.cpp | 17 +-
clang/lib/Sema/SemaExprMember.cpp |2 +
clang/lib/Sema/SemaInit.cpp |5 +
clang/test/Misc/warning-flags-tree.c |4 -
clang/test/Sema/strict-aliasing-warn.c| 192 +++
clang/test/SemaCXX/strict-aliasing-warn.cpp | 1375 +
21 files changed, 1939 insertions(+), 13 deletions(-)
create mode 100644 clang/test/Sema/strict-aliasing-warn.c
create mode 100644 clang/test/SemaCXX/strict-aliasing-warn.cpp
diff --git a/clang/include/clang/AST/ASTConsumer.h
b/clang/include/clang/AST/ASTConsumer.h
index ebcd8059284d8..d5731ed6adf63 100644
--- a/clang/include/clang/AST/ASTConsumer.h
+++ b/clang/include/clang/AST/ASTConsumer.h
@@ -21,6 +21,7 @@ namespace clang {
class DeclGroupRef;
class ASTMutationListener;
class ASTDeserializationListener; // layering violation because void* is ugly
+ class QualType;
class SemaConsumer; // layering violation required for safe SemaConsumer
class TagDecl;
class VarDecl;
@@ -37,6 +38,27 @@ class ASTConsumer {
friend class SemaConsumer;
+public:
+ /// Allow type-based aliasing information to be interrogated by
[clang] [clang] Strict aliasing warning ala GCC [PR50066] (PR #74155)
@@ -128,6 +128,10 @@ class DiagnosticOptions : public
RefCountedBase{
/// whether -Wsystem-headers is enabled on a per-module basis.
std::vector SystemHeaderWarningsModules;
+ /// Level of scrutiny reinterpret_casts get for type-unsafe aliasing
+ /// checks. Requires an ASTConsumer that provides TBAA information.
+ unsigned StrictAliasing;
urnathan wrote:
I was misled by the -Wundef-prefix pattern that I followed. Should have looked
closer and found the DiagnosticOptions.def file.
https://github.com/llvm/llvm-project/pull/74155
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Strict aliasing warning ala GCC [PR50066] (PR #74155)
https://github.com/urnathan edited https://github.com/llvm/llvm-project/pull/74155 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Strict aliasing warning ala GCC [PR50066] (PR #74155)
urnathan wrote: > Thank you for your efforts! I scratched the surface a bit; not qualified to > do an in-depth review. Can you also add a release note? Thanks for your comments. A release note is added. https://github.com/llvm/llvm-project/pull/74155 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Strict aliasing warning ala GCC [PR50066] (PR #74155)
urnathan wrote: > FWIW the GCC doc is > https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wstrict-aliasing_003dn > It says for Level 3 "If optimization is enabled, it also runs in the back > end, where it deals with multiple statement cases using flow-sensitive > points-to information." > > Do you know how it works? Any example? I do not now how it works -- didn't go poking there. Neither do I have examples. https://github.com/llvm/llvm-project/pull/74155 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Strict aliasing warning ala GCC [PR50066] (PR #74155)
urnathan wrote: > Making Sema pull the TBAA info out of clang/lib/CodeGen is a layering > violation (and probably breaks if we aren't actually generating code). If we > need some notion of "aliasing" in Sema, we should pull the relevant code into > clang/lib/AST. That's unfortunate. The code will not call the TBAA stuff if the code generator doesn't provide that predicate -- so if not generating optimized code the warning is inactive. This is the same as with GCC btw. To be clear what's exposed is a new predicate allowing querying of type conversion's TBAAness -- the representations of TBAA etc are not exposed. The CodeGen TBAA machinery builds on llvm's aliasing MDNodes. It would seem a large task to replicate that in AST (and I presume propagating llvm's bits further into AST is even worse?) > I don't have any experience with this warning, so I'm not sure how effective > it is at detecting issues; do a significant fraction of issues actually show > up at a pure syntactic level? Do you have any idea what false positive rate > we should expect? I think at level 3 the false positive rate is very low. Given GCC's extended this over the years from the simple check I had (IIRC any reinterpret_cast between pointers/references to TBAA incompatible types) to the semantics it has now suggests the warning is useful. and the level 3 default is satisfactory. But I don't have hard data. https://github.com/llvm/llvm-project/pull/74155 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Strict aliasing warning ala GCC [PR50066] (PR #74155)
@@ -2026,6 +2027,137 @@ static TryCastResult TryConstCast(Sema ,
ExprResult ,
return TC_Success;
}
+// We're dereferencing E, either by turning into an RValue, or by dereferencing
+// it. Check whether it's a deref of a reinterpret cast that has aliasing
+// issues.
+void Sema::CheckStrictAliasingDeref(Expr const *E, bool IsLValue) {
+ if (Diags.getDiagnosticOptions().StrictAliasing < 3)
+return;
+
+ assert(IsLValue || E->getType()->isAnyPointerType());
+ CastExpr const *CE = nullptr;
+ for (;;) {
+CE = dyn_cast(E->IgnoreParens());
+if (!CE)
+ return;
+
+if (IsLValue || CE->getCastKind() != CK_ArrayToPointerDecay)
+ break;
+
+E = CE->getSubExpr();
+IsLValue = true;
+ }
+
+ if (CE->getCastKind() != (IsLValue ? CK_LValueBitCast : CK_BitCast))
+return;
+
+ if (CE->getSubExpr()->getType()->isVoidPointerType())
+return;
+
+ QualType DestTy = CE->getType();
+ if (!IsLValue)
+DestTy = DestTy->getPointeeType();
+
+ CheckStrictAliasing(CE->getSubExpr(), DestTy, IsLValue,
CE->getSourceRange());
+}
+
+/// We're building a cast from E to pointer type DestType. If ISLValueCast is
+/// true, DestType is the pointer equivalent of the reference type we're
casting
+/// to.
+void Sema::CheckStrictAliasingCast(Expr const *E, QualType DestType,
+ bool IsLValueCast, SourceRange Range) {
+ if (Diags.getDiagnosticOptions().StrictAliasing < 1 ||
urnathan wrote:
Heh, that's amusing :) this started as a switch, and then changed when I found
the switch to be line noise..
https://github.com/llvm/llvm-project/pull/74155
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Strict aliasing warning ala GCC [PR50066] (PR #74155)
@@ -498,3 +498,137 @@
CodeGenTBAA::mergeTBAAInfoForMemoryTransfer(TBAAAccessInfo DestInfo,
// access type regardless of their base types.
return TBAAAccessInfo::getMayAliasInfo();
}
+
+// Determine the aliasing kind bit-converting from type Src to type Dst.
+CodeGenTBAA::AliasingKind CodeGenTBAA::getAliasingKind(QualType ,
+ QualType ) {
+ assert(!Src->isVoidType() && !Dst->isVoidType());
+ if (TypeHasMayAlias(Src) || TypeHasMayAlias(Dst))
+return AK_Ok;
+
+ Src = QualType{Src->getBaseElementTypeUnsafe(), 0};
+ Dst = QualType{Dst->getBaseElementTypeUnsafe(), 0};
+
+ auto *SrcDecl = Src->getAsRecordDecl();
+ auto *DstDecl = Dst->getAsRecordDecl();
+
+ const llvm::MDNode *AnyTBAA = getChar();
+ const llvm::MDNode *SrcTBAA = nullptr;
+ const llvm::MDNode *DstTBAA = nullptr;
+
+ if (!SrcDecl) {
+SrcTBAA = getTypeInfo(Src);
+if (!SrcTBAA || SrcTBAA == AnyTBAA)
+ return AK_Ok;
+ }
+ if (!DstDecl) {
+DstTBAA = getTypeInfo(Dst);
+if (!DstTBAA || DstTBAA == AnyTBAA)
+ return AK_Ok;
+ }
+
+ auto IsAncestor = [](const llvm::MDNode *Ancestor,
+ const llvm::MDNode *Descendant) {
+assert(Ancestor != Descendant && "Identical TBAA");
+while (Descendant->getNumOperands() != 1) {
+ Descendant = cast(Descendant->getOperand(1));
+ if (Descendant == Ancestor)
+return true;
+}
+return false;
+ };
+
+ assert(SrcTBAA != AnyTBAA && DstTBAA != AnyTBAA &&
urnathan wrote:
Ah, I found the current placement better -- as a reminder to the code below
that that particular condition did not need to be handled. The assert got
added explicitly when I needed that reminder!
https://github.com/llvm/llvm-project/pull/74155
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Strict aliasing warning ala GCC [PR50066] (PR #74155)
@@ -498,3 +498,137 @@
CodeGenTBAA::mergeTBAAInfoForMemoryTransfer(TBAAAccessInfo DestInfo,
// access type regardless of their base types.
return TBAAAccessInfo::getMayAliasInfo();
}
+
+// Determine the aliasing kind bit-converting from type Src to type Dst.
+CodeGenTBAA::AliasingKind CodeGenTBAA::getAliasingKind(QualType ,
+ QualType ) {
+ assert(!Src->isVoidType() && !Dst->isVoidType());
+ if (TypeHasMayAlias(Src) || TypeHasMayAlias(Dst))
+return AK_Ok;
+
+ Src = QualType{Src->getBaseElementTypeUnsafe(), 0};
+ Dst = QualType{Dst->getBaseElementTypeUnsafe(), 0};
+
+ auto *SrcDecl = Src->getAsRecordDecl();
+ auto *DstDecl = Dst->getAsRecordDecl();
+
+ const llvm::MDNode *AnyTBAA = getChar();
+ const llvm::MDNode *SrcTBAA = nullptr;
+ const llvm::MDNode *DstTBAA = nullptr;
+
+ if (!SrcDecl) {
+SrcTBAA = getTypeInfo(Src);
+if (!SrcTBAA || SrcTBAA == AnyTBAA)
urnathan wrote:
Yeah, I didn't because it was a class member, but scoped is better. done.
https://github.com/llvm/llvm-project/pull/74155
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Strict aliasing warning ala GCC [PR50066] (PR #74155)
https://github.com/urnathan updated
https://github.com/llvm/llvm-project/pull/74155
>From 89c05618bb75fd073343046f3b54bde5f2254719 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Wed, 15 Nov 2023 15:27:16 -0500
Subject: [PATCH 1/5] [clang] Strict aliasing warning ala GCC [PR50066]
This implements -Wstrict-aliasing(=[123])? along the same lines as
GCC. It's not 100% the same for reasons expanded on below. The default
is level 3, and I have verified that bootstrapping does not trigger
any warnings (just like building with GCC).
As with GCC, higher levels result in fewer warnings, reducing the
number of false positives at the cost of missing (some) potential
cases. Unlike GCC, this is entirely in the FE, we do not propagate any
checking into the IR (so there are cases GCC will detect we do not, I
have not encountered any). GCC's documentation is not very specific
about which cases are detected. I examined GCC's source code to reverse
engineer the algorithm[*], and as can be seen from the testcases, noted
GCC's behaviour.
The strict aliasing check relies on TBAA. LLVM's representation is
semantically different to GCCs for structured types. I have tried to
keep with the spirit of the warning.
The warning checks reinterpret_casts that are CK_BitCast or
CK_LValueBitCast. It also checks C-style casts that are equivalent (to
the extent available, as a C-style bitcast could be a well-formed
static cast).
level=1 looks for reinterpret casts from T * to U * (or an lvalue of
type T to U &), where T and U are not TBAA compatible.
level=2 requires the src expression be an lvalue something of
known(ish) static type. I.e a variable, array dereference or member
access.
level=3 requires level 2 and that the resultant pointer is actually
referenced (lvalue->rvalue or lvalue written). Here we can do better
than GCC, which doesn't represent this in the IR -- we merely get a
dereference (and reference types are modeled as pointers with
auto-dereference semantics). There is one exception to this, which is
by-value aggregate arguments. These end up invoking a copy constructor
(passing a reference of course), but are IMHO morally an rvalue -- so
should trigger.
The warning hooks into clang's code-generator's TBAA machinery. For
scalar types the TBAA is straight forwards, comparing LLVM's MDNode
representaion. For record & union types we check if one of the types
is (recursively) the same (or TBAA compatible) as the first direct
base or a field(s) of the record at offset zero (i.e. the first member
of a record, or any members of a union). This covers both C++ and
C-Style inheritance. Also. the member maybe alias_any, or in
ubiquitous-char's alias set, which is also permissible.
The warning is similar to the alignment check that
CheckCompatibleReinterpretCast does, perhaps some merging could occur
if this is accepted?
[*] I implemented what turned into GCC's level=1 way back when.
WIP: adjust tests
---
clang/include/clang/AST/ASTConsumer.h | 25 +
clang/include/clang/Basic/DiagnosticGroups.td |6 -
clang/include/clang/Basic/DiagnosticOptions.h |4 +
.../clang/Basic/DiagnosticSemaKinds.td|8 +
clang/include/clang/Driver/Options.td |6 +
clang/include/clang/Sema/Sema.h | 11 +
clang/lib/CodeGen/BackendConsumer.h |1 +
clang/lib/CodeGen/CodeGenAction.cpp |4 +
clang/lib/CodeGen/CodeGenModule.h |1 +
clang/lib/CodeGen/CodeGenTBAA.cpp | 134 ++
clang/lib/CodeGen/CodeGenTBAA.h |5 +-
clang/lib/CodeGen/ModuleBuilder.cpp |2 +
clang/lib/Frontend/CompilerInvocation.cpp |3 +
clang/lib/Frontend/FrontendAction.cpp |8 +
clang/lib/Sema/SemaCast.cpp | 139 ++
clang/lib/Sema/SemaExpr.cpp | 17 +-
clang/lib/Sema/SemaExprMember.cpp |2 +
clang/lib/Sema/SemaInit.cpp |5 +
clang/test/Misc/warning-flags-tree.c |4 -
clang/test/Sema/strict-aliasing-warn.c| 192 +++
clang/test/SemaCXX/strict-aliasing-warn.cpp | 1375 +
21 files changed, 1939 insertions(+), 13 deletions(-)
create mode 100644 clang/test/Sema/strict-aliasing-warn.c
create mode 100644 clang/test/SemaCXX/strict-aliasing-warn.cpp
diff --git a/clang/include/clang/AST/ASTConsumer.h
b/clang/include/clang/AST/ASTConsumer.h
index ebcd8059284d8..d5731ed6adf63 100644
--- a/clang/include/clang/AST/ASTConsumer.h
+++ b/clang/include/clang/AST/ASTConsumer.h
@@ -21,6 +21,7 @@ namespace clang {
class DeclGroupRef;
class ASTMutationListener;
class ASTDeserializationListener; // layering violation because void* is ugly
+ class QualType;
class SemaConsumer; // layering violation required for safe SemaConsumer
class TagDecl;
class VarDecl;
@@ -37,6 +38,27 @@ class ASTConsumer {
friend class SemaConsumer;
+public:
+ /// Allow type-based aliasing information to be interrogated by
[clang] [clang] Strict aliasing warning ala GCC [PR50066] (PR #74155)
https://github.com/urnathan edited https://github.com/llvm/llvm-project/pull/74155 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Strict aliasing warning ala GCC [PR50066] (PR #74155)
https://github.com/urnathan ready_for_review https://github.com/llvm/llvm-project/pull/74155 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Avoid recalculating TBAA base type info (PR #73264)
https://github.com/urnathan closed https://github.com/llvm/llvm-project/pull/73264 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Strict aliasing warning ala GCC [PR50066] (PR #74155)
https://github.com/urnathan created https://github.com/llvm/llvm-project/pull/74155 This implements -Wstrict-aliasing(=[123])? along the same lines as GCC. It's not 100% the same for reasons expanded on below. The default is level 3, and I have verified that bootstrapping does not trigger any warnings (just like building with GCC). As with GCC, higher levels result in fewer warnings, reducing the number of false positives at the cost of missing (some) potential cases. Unlike GCC, this is entirely in the FE, we do not propagate any checking into the IR (so there are cases GCC will detect we do not, I have not encountered any). GCC's documentation is not very specific about which cases are detected. I examined GCC's source code to reverse engineer the algorithm[*], and as can be seen from the testcases, noted GCC's behaviour. The strict aliasing check relies on TBAA. LLVM's representation is semantically different to GCCs for structured types. I have tried to keep with the spirit of the warning. The warning checks reinterpret_casts that are CK_BitCast or CK_LValueBitCast. It also checks C-style casts that are equivalent (to the extent available, as a C-style bitcast could be a well-formed static cast). level=1 looks for reinterpret casts from T * to U * (or an lvalue of type T to U &), where T and U are not TBAA compatible. level=2 requires the src expression be an lvalue something of known(ish) static type. I.e a variable, array dereference or member access. level=3 requires level 2 and that the resultant pointer is actually referenced (lvalue->rvalue or lvalue written). Here we can do better than GCC, which doesn't represent this in the IR -- we merely get a dereference (and reference types are modeled as pointers with auto-dereference semantics). There is one exception to this, which is by-value aggregate arguments. These end up invoking a copy constructor (passing a reference of course), but are IMHO morally an rvalue -- so should trigger. The warning hooks into clang's code-generator's TBAA machinery. For scalar types the TBAA is straight forwards, comparing LLVM's MDNode representaion. For record & union types we check if one of the types is (recursively) the same (or TBAA compatible) as the first direct base or a field(s) of the record at offset zero (i.e. the first member of a record, or any members of a union). This covers both C++ and C-Style inheritance. Also. the member maybe alias_any, or in ubiquitous-char's alias set, which is also permissible. The warning is similar to the alignment check that CheckCompatibleReinterpretCast does, perhaps some merging could occur if this is accepted? [*] I implemented what turned into GCC's level=1 way back when. WIP: adjust tests >From 89c05618bb75fd073343046f3b54bde5f2254719 Mon Sep 17 00:00:00 2001 From: Nathan Sidwell Date: Wed, 15 Nov 2023 15:27:16 -0500 Subject: [PATCH] [clang] Strict aliasing warning ala GCC [PR50066] This implements -Wstrict-aliasing(=[123])? along the same lines as GCC. It's not 100% the same for reasons expanded on below. The default is level 3, and I have verified that bootstrapping does not trigger any warnings (just like building with GCC). As with GCC, higher levels result in fewer warnings, reducing the number of false positives at the cost of missing (some) potential cases. Unlike GCC, this is entirely in the FE, we do not propagate any checking into the IR (so there are cases GCC will detect we do not, I have not encountered any). GCC's documentation is not very specific about which cases are detected. I examined GCC's source code to reverse engineer the algorithm[*], and as can be seen from the testcases, noted GCC's behaviour. The strict aliasing check relies on TBAA. LLVM's representation is semantically different to GCCs for structured types. I have tried to keep with the spirit of the warning. The warning checks reinterpret_casts that are CK_BitCast or CK_LValueBitCast. It also checks C-style casts that are equivalent (to the extent available, as a C-style bitcast could be a well-formed static cast). level=1 looks for reinterpret casts from T * to U * (or an lvalue of type T to U &), where T and U are not TBAA compatible. level=2 requires the src expression be an lvalue something of known(ish) static type. I.e a variable, array dereference or member access. level=3 requires level 2 and that the resultant pointer is actually referenced (lvalue->rvalue or lvalue written). Here we can do better than GCC, which doesn't represent this in the IR -- we merely get a dereference (and reference types are modeled as pointers with auto-dereference semantics). There is one exception to this, which is by-value aggregate arguments. These end up invoking a copy constructor (passing a reference of course), but are IMHO morally an rvalue -- so should trigger. The warning hooks into clang's code-generator's TBAA machinery. For scalar types the TBAA is straight forwards, comparing LLVM's
[clang] [clang] Better bitfield access units (PR #65742)
urnathan wrote: You knew this ping was coming, right? https://github.com/llvm/llvm-project/pull/65742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Adjust TBAA Base Info API (PR #73263)
urnathan wrote: sure, like so? https://github.com/llvm/llvm-project/pull/73263 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Adjust TBAA Base Info API (PR #73263)
https://github.com/urnathan updated
https://github.com/llvm/llvm-project/pull/73263
>From 2a312ddadae91ea52b184edaa0d19495c6e0f4a3 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Wed, 22 Nov 2023 20:45:38 -0500
Subject: [PATCH 1/2] [clang][NFC] Adjust TBAA Base Info API
I noticed a couple of minor issues with CodeGenTBAA::getBaseTypeInfo.
1) isValidBaseType explicitly checks for a reference type to return
false, but then also returns false for all non-record types. Just
remove that reference check.
2) All uses of CodeGenTBAA::getBaseTypeInfo from within that class are
when we've already checked the type isValidBaseType. The only case
where this isn't true is from outside the class. It seems better to
have two entry points in this case. Adding a new
'maybeGetBaseTypeInfo' entry point for external uses that returns
nullptr for non valid base types. (Open to other names?)
[This is part one of a pair of changes.]
---
clang/lib/CodeGen/CodeGenModule.cpp | 2 +-
clang/lib/CodeGen/CodeGenTBAA.cpp | 9 +
clang/lib/CodeGen/CodeGenTBAA.h | 11 ---
3 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp
b/clang/lib/CodeGen/CodeGenModule.cpp
index 7cdf50a281cd278..e01fdc3579d88b8 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1318,7 +1318,7 @@ llvm::MDNode *CodeGenModule::getTBAAStructInfo(QualType
QTy) {
llvm::MDNode *CodeGenModule::getTBAABaseTypeInfo(QualType QTy) {
if (!TBAA)
return nullptr;
- return TBAA->getBaseTypeInfo(QTy);
+ return TBAA->maybeGetBaseTypeInfo(QTy);
}
llvm::MDNode *CodeGenModule::getTBAAAccessTagInfo(TBAAAccessInfo Info) {
diff --git a/clang/lib/CodeGen/CodeGenTBAA.cpp
b/clang/lib/CodeGen/CodeGenTBAA.cpp
index 8705d3d65f1a573..9b45a644937b8d9 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.cpp
+++ b/clang/lib/CodeGen/CodeGenTBAA.cpp
@@ -95,8 +95,6 @@ static bool TypeHasMayAlias(QualType QTy) {
/// Check if the given type is a valid base type to be used in access tags.
static bool isValidBaseType(QualType QTy) {
- if (QTy->isReferenceType())
-return false;
if (const RecordType *TTy = QTy->getAs()) {
const RecordDecl *RD = TTy->getDecl()->getDefinition();
// Incomplete types are not valid base access types.
@@ -414,8 +412,7 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfoHelper(const Type
*Ty) {
}
llvm::MDNode *CodeGenTBAA::getBaseTypeInfo(QualType QTy) {
- if (!isValidBaseType(QTy))
-return nullptr;
+ assert(isValidBaseType(QTy) && "Must be a valid base type");
const Type *Ty = Context.getCanonicalType(QTy).getTypePtr();
if (llvm::MDNode *N = BaseTypeMetadataCache[Ty])
@@ -428,6 +425,10 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfo(QualType QTy) {
return BaseTypeMetadataCache[Ty] = TypeNode;
}
+llvm::MDNode *CodeGenTBAA::maybeGetBaseTypeInfo(QualType QTy) {
+ return isValidBaseType(QTy) ? getBaseTypeInfo(QTy) : nullptr;
+}
+
llvm::MDNode *CodeGenTBAA::getAccessTagInfo(TBAAAccessInfo Info) {
assert(!Info.isIncomplete() && "Access to an object of an incomplete type!");
diff --git a/clang/lib/CodeGen/CodeGenTBAA.h b/clang/lib/CodeGen/CodeGenTBAA.h
index a65963596fe9def..53d77e1fefc4352 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.h
+++ b/clang/lib/CodeGen/CodeGenTBAA.h
@@ -166,6 +166,10 @@ class CodeGenTBAA {
/// used to describe accesses to objects of the given base type.
llvm::MDNode *getBaseTypeInfoHelper(const Type *Ty);
+ /// getBaseTypeInfo - Return metadata that describes the given base access
+ /// type. The type must be suitable.
+ llvm::MDNode *getBaseTypeInfo(QualType QTy);
+
public:
CodeGenTBAA(ASTContext , llvm::Module , const CodeGenOptions ,
const LangOptions , MangleContext );
@@ -187,9 +191,10 @@ class CodeGenTBAA {
/// the given type.
llvm::MDNode *getTBAAStructInfo(QualType QTy);
- /// getBaseTypeInfo - Get metadata that describes the given base access type.
- /// Return null if the type is not suitable for use in TBAA access tags.
- llvm::MDNode *getBaseTypeInfo(QualType QTy);
+ /// maybeGetBaseTypeInfo - Get metadata that describes the given base access
+ /// type. Return null if the type is not suitable for use in TBAA access
+ /// tags.
+ llvm::MDNode *maybeGetBaseTypeInfo(QualType QTy);
/// getAccessTagInfo - Get TBAA tag for a given memory access.
llvm::MDNode *getAccessTagInfo(TBAAAccessInfo Info);
>From f4755f32880748100d0a3aa93836341033418f4d Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Fri, 1 Dec 2023 16:08:04 -0500
Subject: [PATCH 2/2] Rename internal getter to getValidBaseTypeInfo
---
clang/lib/CodeGen/CodeGenModule.cpp | 2 +-
clang/lib/CodeGen/CodeGenTBAA.cpp | 18 ++
clang/lib/CodeGen/CodeGenTBAA.h | 10 +-
3 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp
b/clang/lib/CodeGen/CodeGenModule.cpp
index
[clang] [clang] Avoid recalculating TBAA base type info (PR #73264)
@@ -418,14 +418,20 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfo(QualType QTy) {
return nullptr;
const Type *Ty = Context.getCanonicalType(QTy).getTypePtr();
- if (llvm::MDNode *N = BaseTypeMetadataCache[Ty])
-return N;
- // Note that the following helper call is allowed to add new nodes to the
- // cache, which invalidates all its previously obtained iterators. So we
- // first generate the node for the type and then add that node to the cache.
+ // nullptr is a valid value in the cache, so use find rather than []
+ auto I = BaseTypeMetadataCache.find(Ty);
+ if (I != BaseTypeMetadataCache.end())
+return I->second;
+
+ // First calculate the metadata, before recomputing the insertion point, as
+ // the helper can recursively call us.
llvm::MDNode *TypeNode = getBaseTypeInfoHelper(Ty);
- return BaseTypeMetadataCache[Ty] = TypeNode;
+ LLVM_ATTRIBUTE_UNUSED auto inserted =
+ BaseTypeMetadataCache.try_emplace(Ty, TypeNode);
urnathan wrote:
done, will push when tests complete
https://github.com/llvm/llvm-project/pull/73264
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Avoid recalculating TBAA base type info (PR #73264)
https://github.com/urnathan updated
https://github.com/llvm/llvm-project/pull/73264
>From e4c92cd687782743ba939becf7ea8862eea6a108 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Thu, 23 Nov 2023 11:30:12 -0500
Subject: [PATCH 1/2] [clang] Avoid recalculating TBAA base type info
I noticed that TBAA BaseTypeMetadataCache can legitimately store null
values, but it also uses that to mean 'no entry'. Thus nullptr entries
get continually recalculated. (AFAICT null entries can never become
non-null.) This changes the hash lookup/insertion to use find and
try_emplace rather than the subscript operator.
While there, getBaseTypeInfoHelper will insert non-null values into
the hashtable itself, but simply return nullptr values. The only
caller, getBaseTypeInfo unconditionally inserts the value anyway. It
seems clearer to let getBaseTypeInfo do the insertion and
getBaseTypeInfoHelper merely computes.
---
clang/lib/CodeGen/CodeGenTBAA.cpp | 24 +++-
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenTBAA.cpp
b/clang/lib/CodeGen/CodeGenTBAA.cpp
index 8705d3d65f1a573..94dc304bfc243b9 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.cpp
+++ b/clang/lib/CodeGen/CodeGenTBAA.cpp
@@ -342,7 +342,7 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfoHelper(const Type
*Ty) {
// field. Virtual bases are more complex and omitted, but avoid an
// incomplete view for NewStructPathTBAA.
if (CodeGenOpts.NewStructPathTBAA && CXXRD->getNumVBases() != 0)
-return BaseTypeMetadataCache[Ty] = nullptr;
+return nullptr;
for (const CXXBaseSpecifier : CXXRD->bases()) {
if (B.isVirtual())
continue;
@@ -354,7 +354,7 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfoHelper(const Type
*Ty) {
? getBaseTypeInfo(BaseQTy)
: getTypeInfo(BaseQTy);
if (!TypeNode)
- return BaseTypeMetadataCache[Ty] = nullptr;
+ return nullptr;
uint64_t Offset = Layout.getBaseClassOffset(BaseRD).getQuantity();
uint64_t Size =
Context.getASTRecordLayout(BaseRD).getDataSize().getQuantity();
@@ -378,7 +378,7 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfoHelper(const Type
*Ty) {
llvm::MDNode *TypeNode = isValidBaseType(FieldQTy) ?
getBaseTypeInfo(FieldQTy) : getTypeInfo(FieldQTy);
if (!TypeNode)
-return BaseTypeMetadataCache[Ty] = nullptr;
+return nullptr;
uint64_t BitOffset = Layout.getFieldOffset(Field->getFieldIndex());
uint64_t Offset = Context.toCharUnitsFromBits(BitOffset).getQuantity();
@@ -418,14 +418,20 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfo(QualType QTy) {
return nullptr;
const Type *Ty = Context.getCanonicalType(QTy).getTypePtr();
- if (llvm::MDNode *N = BaseTypeMetadataCache[Ty])
-return N;
- // Note that the following helper call is allowed to add new nodes to the
- // cache, which invalidates all its previously obtained iterators. So we
- // first generate the node for the type and then add that node to the cache.
+ // nullptr is a valid value in the cache, so use find rather than []
+ auto I = BaseTypeMetadataCache.find(Ty);
+ if (I != BaseTypeMetadataCache.end())
+return I->second;
+
+ // First calculate the metadata, before recomputing the insertion point, as
+ // the helper can recursively call us.
llvm::MDNode *TypeNode = getBaseTypeInfoHelper(Ty);
- return BaseTypeMetadataCache[Ty] = TypeNode;
+ LLVM_ATTRIBUTE_UNUSED auto inserted =
+ BaseTypeMetadataCache.try_emplace(Ty, TypeNode);
+ assert(inserted.second && "BaseType metadata was already inserted");
+
+ return TypeNode;
}
llvm::MDNode *CodeGenTBAA::getAccessTagInfo(TBAAAccessInfo Info) {
>From fee3b7407ab6fe73476eed82e387cceb9b6ef88c Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Fri, 1 Dec 2023 15:58:51 -0500
Subject: [PATCH 2/2] Use insert not try_emplace
---
clang/lib/CodeGen/CodeGenTBAA.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/CodeGen/CodeGenTBAA.cpp
b/clang/lib/CodeGen/CodeGenTBAA.cpp
index 94dc304bfc243b9..5906b14dd93cf08 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.cpp
+++ b/clang/lib/CodeGen/CodeGenTBAA.cpp
@@ -428,7 +428,7 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfo(QualType QTy) {
// the helper can recursively call us.
llvm::MDNode *TypeNode = getBaseTypeInfoHelper(Ty);
LLVM_ATTRIBUTE_UNUSED auto inserted =
- BaseTypeMetadataCache.try_emplace(Ty, TypeNode);
+ BaseTypeMetadataCache.insert({Ty, TypeNode});
assert(inserted.second && "BaseType metadata was already inserted");
return TypeNode;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Avoid recalculating TBAA base type info (PR #73264)
https://github.com/urnathan ready_for_review https://github.com/llvm/llvm-project/pull/73264 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Adjust TBAA Base Info API (PR #73263)
https://github.com/urnathan ready_for_review https://github.com/llvm/llvm-project/pull/73263 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Refactor TBAA Base Info construction (PR #70499)
https://github.com/urnathan closed https://github.com/llvm/llvm-project/pull/70499 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Avoid recalculating TBAA base type info (PR #73264)
https://github.com/urnathan created
https://github.com/llvm/llvm-project/pull/73264
I noticed that TBAA BaseTypeMetadataCache can legitimately store null values,
but it also uses that to mean 'no entry'. Thus nullptr entries get continually
recalculated. (AFAICT null entries can never become non-null.) This changes
the hash lookup/insertion to use find and try_emplace rather than the subscript
operator.
While there, getBaseTypeInfoHelper will insert non-null values into the
hashtable itself, but simply return nullptr values. The only caller,
getBaseTypeInfo unconditionally inserts the value anyway. It seems clearer to
let getBaseTypeInfo do the insertion and getBaseTypeInfoHelper merely computes.
[This is the second of a pair of prs]
>From e4c92cd687782743ba939becf7ea8862eea6a108 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Thu, 23 Nov 2023 11:30:12 -0500
Subject: [PATCH] [clang] Avoid recalculating TBAA base type info
I noticed that TBAA BaseTypeMetadataCache can legitimately store null
values, but it also uses that to mean 'no entry'. Thus nullptr entries
get continually recalculated. (AFAICT null entries can never become
non-null.) This changes the hash lookup/insertion to use find and
try_emplace rather than the subscript operator.
While there, getBaseTypeInfoHelper will insert non-null values into
the hashtable itself, but simply return nullptr values. The only
caller, getBaseTypeInfo unconditionally inserts the value anyway. It
seems clearer to let getBaseTypeInfo do the insertion and
getBaseTypeInfoHelper merely computes.
---
clang/lib/CodeGen/CodeGenTBAA.cpp | 24 +++-
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenTBAA.cpp
b/clang/lib/CodeGen/CodeGenTBAA.cpp
index 8705d3d65f1a573..94dc304bfc243b9 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.cpp
+++ b/clang/lib/CodeGen/CodeGenTBAA.cpp
@@ -342,7 +342,7 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfoHelper(const Type
*Ty) {
// field. Virtual bases are more complex and omitted, but avoid an
// incomplete view for NewStructPathTBAA.
if (CodeGenOpts.NewStructPathTBAA && CXXRD->getNumVBases() != 0)
-return BaseTypeMetadataCache[Ty] = nullptr;
+return nullptr;
for (const CXXBaseSpecifier : CXXRD->bases()) {
if (B.isVirtual())
continue;
@@ -354,7 +354,7 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfoHelper(const Type
*Ty) {
? getBaseTypeInfo(BaseQTy)
: getTypeInfo(BaseQTy);
if (!TypeNode)
- return BaseTypeMetadataCache[Ty] = nullptr;
+ return nullptr;
uint64_t Offset = Layout.getBaseClassOffset(BaseRD).getQuantity();
uint64_t Size =
Context.getASTRecordLayout(BaseRD).getDataSize().getQuantity();
@@ -378,7 +378,7 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfoHelper(const Type
*Ty) {
llvm::MDNode *TypeNode = isValidBaseType(FieldQTy) ?
getBaseTypeInfo(FieldQTy) : getTypeInfo(FieldQTy);
if (!TypeNode)
-return BaseTypeMetadataCache[Ty] = nullptr;
+return nullptr;
uint64_t BitOffset = Layout.getFieldOffset(Field->getFieldIndex());
uint64_t Offset = Context.toCharUnitsFromBits(BitOffset).getQuantity();
@@ -418,14 +418,20 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfo(QualType QTy) {
return nullptr;
const Type *Ty = Context.getCanonicalType(QTy).getTypePtr();
- if (llvm::MDNode *N = BaseTypeMetadataCache[Ty])
-return N;
- // Note that the following helper call is allowed to add new nodes to the
- // cache, which invalidates all its previously obtained iterators. So we
- // first generate the node for the type and then add that node to the cache.
+ // nullptr is a valid value in the cache, so use find rather than []
+ auto I = BaseTypeMetadataCache.find(Ty);
+ if (I != BaseTypeMetadataCache.end())
+return I->second;
+
+ // First calculate the metadata, before recomputing the insertion point, as
+ // the helper can recursively call us.
llvm::MDNode *TypeNode = getBaseTypeInfoHelper(Ty);
- return BaseTypeMetadataCache[Ty] = TypeNode;
+ LLVM_ATTRIBUTE_UNUSED auto inserted =
+ BaseTypeMetadataCache.try_emplace(Ty, TypeNode);
+ assert(inserted.second && "BaseType metadata was already inserted");
+
+ return TypeNode;
}
llvm::MDNode *CodeGenTBAA::getAccessTagInfo(TBAAAccessInfo Info) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Adjust TBAA Base Info API (PR #73263)
https://github.com/urnathan created
https://github.com/llvm/llvm-project/pull/73263
I noticed a couple of minor issues with CodeGenTBAA::getBaseTypeInfo.
1) isValidBaseType explicitly checks for a reference type to return false, but
then also returns false for all non-record types. Just remove that reference
check.
2) All uses of CodeGenTBAA::getBaseTypeInfo from within that class are when
we've already checked the type isValidBaseType. The only case where this isn't
true is from outside the class. It seems better to have two entry points in
this case. Adding a new
'maybeGetBaseTypeInfo' entry point for external uses that returns nullptr for
non valid base types. (Open to other names?)
[This is part one of a pair of changes.]
>From 2a312ddadae91ea52b184edaa0d19495c6e0f4a3 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Wed, 22 Nov 2023 20:45:38 -0500
Subject: [PATCH] [clang][NFC] Adjust TBAA Base Info API
I noticed a couple of minor issues with CodeGenTBAA::getBaseTypeInfo.
1) isValidBaseType explicitly checks for a reference type to return
false, but then also returns false for all non-record types. Just
remove that reference check.
2) All uses of CodeGenTBAA::getBaseTypeInfo from within that class are
when we've already checked the type isValidBaseType. The only case
where this isn't true is from outside the class. It seems better to
have two entry points in this case. Adding a new
'maybeGetBaseTypeInfo' entry point for external uses that returns
nullptr for non valid base types. (Open to other names?)
[This is part one of a pair of changes.]
---
clang/lib/CodeGen/CodeGenModule.cpp | 2 +-
clang/lib/CodeGen/CodeGenTBAA.cpp | 9 +
clang/lib/CodeGen/CodeGenTBAA.h | 11 ---
3 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp
b/clang/lib/CodeGen/CodeGenModule.cpp
index 7cdf50a281cd278..e01fdc3579d88b8 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1318,7 +1318,7 @@ llvm::MDNode *CodeGenModule::getTBAAStructInfo(QualType
QTy) {
llvm::MDNode *CodeGenModule::getTBAABaseTypeInfo(QualType QTy) {
if (!TBAA)
return nullptr;
- return TBAA->getBaseTypeInfo(QTy);
+ return TBAA->maybeGetBaseTypeInfo(QTy);
}
llvm::MDNode *CodeGenModule::getTBAAAccessTagInfo(TBAAAccessInfo Info) {
diff --git a/clang/lib/CodeGen/CodeGenTBAA.cpp
b/clang/lib/CodeGen/CodeGenTBAA.cpp
index 8705d3d65f1a573..9b45a644937b8d9 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.cpp
+++ b/clang/lib/CodeGen/CodeGenTBAA.cpp
@@ -95,8 +95,6 @@ static bool TypeHasMayAlias(QualType QTy) {
/// Check if the given type is a valid base type to be used in access tags.
static bool isValidBaseType(QualType QTy) {
- if (QTy->isReferenceType())
-return false;
if (const RecordType *TTy = QTy->getAs()) {
const RecordDecl *RD = TTy->getDecl()->getDefinition();
// Incomplete types are not valid base access types.
@@ -414,8 +412,7 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfoHelper(const Type
*Ty) {
}
llvm::MDNode *CodeGenTBAA::getBaseTypeInfo(QualType QTy) {
- if (!isValidBaseType(QTy))
-return nullptr;
+ assert(isValidBaseType(QTy) && "Must be a valid base type");
const Type *Ty = Context.getCanonicalType(QTy).getTypePtr();
if (llvm::MDNode *N = BaseTypeMetadataCache[Ty])
@@ -428,6 +425,10 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfo(QualType QTy) {
return BaseTypeMetadataCache[Ty] = TypeNode;
}
+llvm::MDNode *CodeGenTBAA::maybeGetBaseTypeInfo(QualType QTy) {
+ return isValidBaseType(QTy) ? getBaseTypeInfo(QTy) : nullptr;
+}
+
llvm::MDNode *CodeGenTBAA::getAccessTagInfo(TBAAAccessInfo Info) {
assert(!Info.isIncomplete() && "Access to an object of an incomplete type!");
diff --git a/clang/lib/CodeGen/CodeGenTBAA.h b/clang/lib/CodeGen/CodeGenTBAA.h
index a65963596fe9def..53d77e1fefc4352 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.h
+++ b/clang/lib/CodeGen/CodeGenTBAA.h
@@ -166,6 +166,10 @@ class CodeGenTBAA {
/// used to describe accesses to objects of the given base type.
llvm::MDNode *getBaseTypeInfoHelper(const Type *Ty);
+ /// getBaseTypeInfo - Return metadata that describes the given base access
+ /// type. The type must be suitable.
+ llvm::MDNode *getBaseTypeInfo(QualType QTy);
+
public:
CodeGenTBAA(ASTContext , llvm::Module , const CodeGenOptions ,
const LangOptions , MangleContext );
@@ -187,9 +191,10 @@ class CodeGenTBAA {
/// the given type.
llvm::MDNode *getTBAAStructInfo(QualType QTy);
- /// getBaseTypeInfo - Get metadata that describes the given base access type.
- /// Return null if the type is not suitable for use in TBAA access tags.
- llvm::MDNode *getBaseTypeInfo(QualType QTy);
+ /// maybeGetBaseTypeInfo - Get metadata that describes the given base access
+ /// type. Return null if the type is not suitable for use in TBAA access
+ /// tags.
+ llvm::MDNode *maybeGetBaseTypeInfo(QualType QTy);
[clang] [clang] Refactor TBAA Base Info construction (PR #70499)
urnathan wrote: I'm going to break this apart, as I've realized this has conflated two separate problems. https://github.com/llvm/llvm-project/pull/70499 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] Fix python escapes (PR #71170)
https://github.com/urnathan closed https://github.com/llvm/llvm-project/pull/71170 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Better bitfield access units (PR #65742)
urnathan wrote: thanks @yonghong-song! Taking the opportunity for a ping :) https://github.com/llvm/llvm-project/pull/65742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] Fix python escapes (PR #71170)
https://github.com/urnathan ready_for_review https://github.com/llvm/llvm-project/pull/71170 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] Fix python escapes (PR #71170)
https://github.com/urnathan edited https://github.com/llvm/llvm-project/pull/71170 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] Fix python escapes (PR #71170)
https://github.com/urnathan updated
https://github.com/llvm/llvm-project/pull/71170
>From 9b5cb1ac8d4b9a2aaa4c06e41620e38b6c3cae8c Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Thu, 2 Nov 2023 18:14:05 -0400
Subject: [PATCH 1/3] Fix python escapes
With Fedora 39, I encountered 2 new python warnings:
/home/nathan/gh/llvm/push/strict-aliasing/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py:28:
SyntaxWarning: invalid escape sequence '\*'
self.implementationContent += """
/home/nathan/gh/llvm/push/strict-aliasing/llvm/test/lit.cfg.py:274:
SyntaxWarning: invalid escape sequence '\d'
match = re.search("release (\d+)\.(\d+)", ptxas_out)
Use raw strings here. I guess python got pickier or something?
May as well fix the blank line caused by """NEWLINE
...
"""
---
clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py | 4 ++--
llvm/test/lit.cfg.py| 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
b/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
index dafb332961ede86..54cfd0741f9d122 100755
--- a/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
+++ b/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
@@ -25,7 +25,7 @@ def __init__(self, templateClasses):
def GeneratePrologue(self):
-self.implementationContent += """
+self.implementationContent += R"""
/*===- Generated file ---*- C++
-*-===*\
|*
*|
|* Introspection of available AST node SourceLocations
*|
@@ -58,7 +58,7 @@ def GeneratePrologue(self):
private:
std::vector
};
-"""
+"""[1:]
def GenerateBaseGetLocationsDeclaration(self, CladeName):
InstanceDecoration = "*"
diff --git a/llvm/test/lit.cfg.py b/llvm/test/lit.cfg.py
index ab245b71cdd16a5..cf050bbfe3b1413 100644
--- a/llvm/test/lit.cfg.py
+++ b/llvm/test/lit.cfg.py
@@ -271,7 +271,7 @@ def ptxas_version(ptxas):
ptxas_cmd = subprocess.Popen([ptxas, "--version"], stdout=subprocess.PIPE)
ptxas_out = ptxas_cmd.stdout.read().decode("ascii")
ptxas_cmd.wait()
-match = re.search("release (\d+)\.(\d+)", ptxas_out)
+match = re.search(R"release (\d+)\.(\d+)", ptxas_out)
if match:
return (int(match.group(1)), int(match.group(2)))
print("couldn't determine ptxas version")
>From 0cf6eb5b293752525cace1dee1ba26e143386809 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Fri, 3 Nov 2023 07:44:11 -0400
Subject: [PATCH 2/3] Lower case raw string
---
clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py | 2 +-
llvm/test/lit.cfg.py| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
b/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
index 54cfd0741f9d122..a6843f70adedae9 100755
--- a/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
+++ b/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
@@ -25,7 +25,7 @@ def __init__(self, templateClasses):
def GeneratePrologue(self):
-self.implementationContent += R"""
+self.implementationContent += r"""
/*===- Generated file ---*- C++
-*-===*\
|*
*|
|* Introspection of available AST node SourceLocations
*|
diff --git a/llvm/test/lit.cfg.py b/llvm/test/lit.cfg.py
index cf050bbfe3b1413..5f4cff424f073b8 100644
--- a/llvm/test/lit.cfg.py
+++ b/llvm/test/lit.cfg.py
@@ -271,7 +271,7 @@ def ptxas_version(ptxas):
ptxas_cmd = subprocess.Popen([ptxas, "--version"], stdout=subprocess.PIPE)
ptxas_out = ptxas_cmd.stdout.read().decode("ascii")
ptxas_cmd.wait()
-match = re.search(R"release (\d+)\.(\d+)", ptxas_out)
+match = re.search(r"release (\d+)\.(\d+)", ptxas_out)
if match:
return (int(match.group(1)), int(match.group(2)))
print("couldn't determine ptxas version")
>From c483141c81052828a1a3793377b0bffdb2300a65 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Fri, 3 Nov 2023 08:18:33 -0400
Subject: [PATCH 3/3] lose [1:]
---
clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
b/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
index a6843f70adedae9..7671f9691c09610 100755
--- a/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
+++ b/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
@@ -58,7 +58,7 @@ def GeneratePrologue(self):
private:
std::vector
};
-"""[1:]
+"""
def GenerateBaseGetLocationsDeclaration(self, CladeName):
InstanceDecoration = "*"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
[llvm] [clang] Fix python escapes (PR #71170)
https://github.com/urnathan updated
https://github.com/llvm/llvm-project/pull/71170
>From 9b5cb1ac8d4b9a2aaa4c06e41620e38b6c3cae8c Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Thu, 2 Nov 2023 18:14:05 -0400
Subject: [PATCH 1/2] Fix python escapes
With Fedora 39, I encountered 2 new python warnings:
/home/nathan/gh/llvm/push/strict-aliasing/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py:28:
SyntaxWarning: invalid escape sequence '\*'
self.implementationContent += """
/home/nathan/gh/llvm/push/strict-aliasing/llvm/test/lit.cfg.py:274:
SyntaxWarning: invalid escape sequence '\d'
match = re.search("release (\d+)\.(\d+)", ptxas_out)
Use raw strings here. I guess python got pickier or something?
May as well fix the blank line caused by """NEWLINE
...
"""
---
clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py | 4 ++--
llvm/test/lit.cfg.py| 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
b/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
index dafb332961ede86..54cfd0741f9d122 100755
--- a/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
+++ b/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
@@ -25,7 +25,7 @@ def __init__(self, templateClasses):
def GeneratePrologue(self):
-self.implementationContent += """
+self.implementationContent += R"""
/*===- Generated file ---*- C++
-*-===*\
|*
*|
|* Introspection of available AST node SourceLocations
*|
@@ -58,7 +58,7 @@ def GeneratePrologue(self):
private:
std::vector
};
-"""
+"""[1:]
def GenerateBaseGetLocationsDeclaration(self, CladeName):
InstanceDecoration = "*"
diff --git a/llvm/test/lit.cfg.py b/llvm/test/lit.cfg.py
index ab245b71cdd16a5..cf050bbfe3b1413 100644
--- a/llvm/test/lit.cfg.py
+++ b/llvm/test/lit.cfg.py
@@ -271,7 +271,7 @@ def ptxas_version(ptxas):
ptxas_cmd = subprocess.Popen([ptxas, "--version"], stdout=subprocess.PIPE)
ptxas_out = ptxas_cmd.stdout.read().decode("ascii")
ptxas_cmd.wait()
-match = re.search("release (\d+)\.(\d+)", ptxas_out)
+match = re.search(R"release (\d+)\.(\d+)", ptxas_out)
if match:
return (int(match.group(1)), int(match.group(2)))
print("couldn't determine ptxas version")
>From 0cf6eb5b293752525cace1dee1ba26e143386809 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Fri, 3 Nov 2023 07:44:11 -0400
Subject: [PATCH 2/2] Lower case raw string
---
clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py | 2 +-
llvm/test/lit.cfg.py| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
b/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
index 54cfd0741f9d122..a6843f70adedae9 100755
--- a/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
+++ b/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
@@ -25,7 +25,7 @@ def __init__(self, templateClasses):
def GeneratePrologue(self):
-self.implementationContent += R"""
+self.implementationContent += r"""
/*===- Generated file ---*- C++
-*-===*\
|*
*|
|* Introspection of available AST node SourceLocations
*|
diff --git a/llvm/test/lit.cfg.py b/llvm/test/lit.cfg.py
index cf050bbfe3b1413..5f4cff424f073b8 100644
--- a/llvm/test/lit.cfg.py
+++ b/llvm/test/lit.cfg.py
@@ -271,7 +271,7 @@ def ptxas_version(ptxas):
ptxas_cmd = subprocess.Popen([ptxas, "--version"], stdout=subprocess.PIPE)
ptxas_out = ptxas_cmd.stdout.read().decode("ascii")
ptxas_cmd.wait()
-match = re.search(R"release (\d+)\.(\d+)", ptxas_out)
+match = re.search(r"release (\d+)\.(\d+)", ptxas_out)
if match:
return (int(match.group(1)), int(match.group(2)))
print("couldn't determine ptxas version")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] Fix python escapes (PR #71170)
https://github.com/urnathan created
https://github.com/llvm/llvm-project/pull/71170
With Fedora 39, I encountered 2 new python warnings:
/home/nathan/gh/llvm/push/strict-aliasing/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py:28:
SyntaxWarning: invalid escape sequence '\*'
self.implementationContent += """
/home/nathan/gh/llvm/push/strict-aliasing/llvm/test/lit.cfg.py:274:
SyntaxWarning: invalid escape sequence '\d'
match = re.search("release (\d+)\.(\d+)", ptxas_out)
Use raw strings here. I guess python got pickier or something?
While there, might as well fix the blank line caused by """NEWLINE ...
""" by dropping its first char.
>From 9b5cb1ac8d4b9a2aaa4c06e41620e38b6c3cae8c Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Thu, 2 Nov 2023 18:14:05 -0400
Subject: [PATCH] Fix python escapes
With Fedora 39, I encountered 2 new python warnings:
/home/nathan/gh/llvm/push/strict-aliasing/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py:28:
SyntaxWarning: invalid escape sequence '\*'
self.implementationContent += """
/home/nathan/gh/llvm/push/strict-aliasing/llvm/test/lit.cfg.py:274:
SyntaxWarning: invalid escape sequence '\d'
match = re.search("release (\d+)\.(\d+)", ptxas_out)
Use raw strings here. I guess python got pickier or something?
May as well fix the blank line caused by """NEWLINE
...
"""
---
clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py | 4 ++--
llvm/test/lit.cfg.py| 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
b/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
index dafb332961ede86..54cfd0741f9d122 100755
--- a/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
+++ b/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
@@ -25,7 +25,7 @@ def __init__(self, templateClasses):
def GeneratePrologue(self):
-self.implementationContent += """
+self.implementationContent += R"""
/*===- Generated file ---*- C++
-*-===*\
|*
*|
|* Introspection of available AST node SourceLocations
*|
@@ -58,7 +58,7 @@ def GeneratePrologue(self):
private:
std::vector
};
-"""
+"""[1:]
def GenerateBaseGetLocationsDeclaration(self, CladeName):
InstanceDecoration = "*"
diff --git a/llvm/test/lit.cfg.py b/llvm/test/lit.cfg.py
index ab245b71cdd16a5..cf050bbfe3b1413 100644
--- a/llvm/test/lit.cfg.py
+++ b/llvm/test/lit.cfg.py
@@ -271,7 +271,7 @@ def ptxas_version(ptxas):
ptxas_cmd = subprocess.Popen([ptxas, "--version"], stdout=subprocess.PIPE)
ptxas_out = ptxas_cmd.stdout.read().decode("ascii")
ptxas_cmd.wait()
-match = re.search("release (\d+)\.(\d+)", ptxas_out)
+match = re.search(R"release (\d+)\.(\d+)", ptxas_out)
if match:
return (int(match.group(1)), int(match.group(2)))
print("couldn't determine ptxas version")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Better bitfield access units (PR #65742)
urnathan wrote: > One very brief note: in the comments in the code, you might want to > distinguish between the semantic width of a bitfield (i.e. the C standard > notion of a "memory location", which has ABI significance), vs. the accesses > we choose to generate (we don't need to generate no-op reads/writes). Indeed, the naming here is unfortunately overloaded. SysV psABIs use 'Storage Unit' (at least 86 does, and IIRC others follow the same nomenclature). But Clang also uses 'StorageInfo' and 'Storage$FOO' in its bitfield structures, which is unfortunate. I've used 'Access Unit' to describe the latter in this patch. If you meant something else, please clarify (or if I've missed some places?). https://github.com/llvm/llvm-project/pull/65742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Refactor TBAA Base Info construction (PR #70499)
https://github.com/urnathan ready_for_review https://github.com/llvm/llvm-project/pull/70499 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Robustify openmp test (PR #69739)
https://github.com/urnathan closed https://github.com/llvm/llvm-project/pull/69739 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Assert not llvm_unreachable (PR #70149)
https://github.com/urnathan closed https://github.com/llvm/llvm-project/pull/70149 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Robustify openmp test (PR #69739)
urnathan wrote: @jdoerfert thanks for that, here's an update that focusses the check-not more precisely. https://github.com/llvm/llvm-project/pull/69739 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Robustify openmp test (PR #69739)
https://github.com/urnathan updated
https://github.com/llvm/llvm-project/pull/69739
>From bb391aa466577f4187af6b284ee5107090778a03 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Fri, 20 Oct 2023 11:43:08 -0400
Subject: [PATCH 1/2] [clang] Robustify open mp test
If the source path contains 'alias' this would spuriously fail. Be
more specific about not wanting [no]alias annotations.
---
clang/test/OpenMP/declare_variant_device_kind_codegen.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/test/OpenMP/declare_variant_device_kind_codegen.cpp
b/clang/test/OpenMP/declare_variant_device_kind_codegen.cpp
index daa14f1e3a93129..aec71bd5b5da20e 100644
--- a/clang/test/OpenMP/declare_variant_device_kind_codegen.cpp
+++ b/clang/test/OpenMP/declare_variant_device_kind_codegen.cpp
@@ -80,7 +80,7 @@
// expected-no-diagnostics
-// CHECK-NOT: alias
+// CHECK-NOT: {{ (no)?alias }}
// CHECK-NOT: ret i32 {{1|4|81|84}}
// CHECK-DAG: declare {{.*}}i32 @_Z5bazzzv()
>From 47ccce0676a39eb64e5305325d88b9b101e03a26 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Fri, 27 Oct 2023 16:38:11 -0400
Subject: [PATCH 2/2] [clang] Robustify open mp test
If the source path contains 'alias' this would spuriously fail. Be
more specific about not wanting global aliases, which is what
introduced this check.
---
clang/test/OpenMP/declare_variant_device_kind_codegen.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/test/OpenMP/declare_variant_device_kind_codegen.cpp
b/clang/test/OpenMP/declare_variant_device_kind_codegen.cpp
index aec71bd5b5da20e..4f9a86f1e0080d9 100644
--- a/clang/test/OpenMP/declare_variant_device_kind_codegen.cpp
+++ b/clang/test/OpenMP/declare_variant_device_kind_codegen.cpp
@@ -80,7 +80,8 @@
// expected-no-diagnostics
-// CHECK-NOT: {{ (no)?alias }}
+// Verify no unexpected global symbol aliasing
+// CHECK-NOT: @{{[^ ]+}} = {{.*}}alias
// CHECK-NOT: ret i32 {{1|4|81|84}}
// CHECK-DAG: declare {{.*}}i32 @_Z5bazzzv()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
