[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 the
[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 &Dst,
+ QualType &Src) {
+ 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)
@@ -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 &Dst,
+ QualType &Src) {
+ 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)
@@ -2026,6 +2027,137 @@ static TryCastResult TryConstCast(Sema &Self,
ExprResult &SrcExpr,
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)
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)
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: > 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)
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)
@@ -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 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 the
[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 *)(&ScalarINT));
+ // level12-note@-1{{not alias compatible}}
+
+ // GCC:
+ ByPtr((LONG *)((void *)(&ScalarINT)));
+
+ // 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}}
+}
+
+// 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 *)(&Ary[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 *)(&Struct.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 *)(&(&Struct)->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 *)(&ScalarINT));
+ // GCC:
+ ByPtr((UINT *)((void *)(&ScalarINT)));
+ // GCC:
+ ByVal(*(UINT *)(&ScalarINT));
+}
+
+// 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 *)&in);
+ ByValOuter2(*(struct Outer2 *)&i);
+ ByValInner(*(struct Inner *)&o.o);
+ ByValInner(*(struct Inner *)&o);
+ ByVal(*(int *)&o);
+}
+
+// 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 *)&d;
+ // level123-note@-1{{not alias compatible}}
+
+ // GCC: 1, 2, 3
+ // level23-warning@+2{{type-punned pointer breaks}}
+ // level1-warnin
[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] 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] 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 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] 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][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: 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 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] 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
[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
[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][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)
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] 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] 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
[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 &TLRG;
};
-"""
+"""[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
[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 &TLRG;
};
-"""
+"""[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 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 &TLRG;
};
-"""
+"""[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 &TLRG;
};
-"""[1:]
+"""
def GenerateBaseGetLocationsDeclaration(self, CladeName):
InstanceDecoration = "*"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm
[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 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
[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] 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 &Dst,
+ QualType &Src) {
+ 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)
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(&obj) = 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][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 &Ctx, llvm::Module &M, const CodeGenOptions &CGO,
const LangOptions &Features, MangleContext &MContext);
@@ -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 *maybeGetBaseTyp
[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 &B : 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] 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][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] 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] 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 &B : 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)
@@ -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][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 &Ctx, llvm::Module &M, const CodeGenOptions &CGO,
const LangOptions &Features, MangleContext &MContext);
@@ -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 e01f
[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] 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] 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 MD
[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 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] 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][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] 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: @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: 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)
@@ -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] de867c6 - [clang] Reformat
Author: Nathan Sidwell
Date: 2022-03-24T05:56:23-07:00
New Revision: de867c6d6ed86851dcd0f9de1154db3d42a0ad63
URL:
https://github.com/llvm/llvm-project/commit/de867c6d6ed86851dcd0f9de1154db3d42a0ad63
DIFF:
https://github.com/llvm/llvm-project/commit/de867c6d6ed86851dcd0f9de1154db3d42a0ad63.diff
LOG: [clang] Reformat
Reformat some misindentation that is coincidentally close to a piece
being worked on.
Reviewed By: dblaikie
Differential Revision: https://reviews.llvm.org/D122314
Added:
Modified:
clang/lib/Index/USRGeneration.cpp
Removed:
diff --git a/clang/lib/Index/USRGeneration.cpp
b/clang/lib/Index/USRGeneration.cpp
index 41edd431dd5b8..129114ebfaedc 100644
--- a/clang/lib/Index/USRGeneration.cpp
+++ b/clang/lib/Index/USRGeneration.cpp
@@ -549,22 +549,22 @@ void USRGenerator::VisitTagDecl(const TagDecl *D) {
if (const TypedefNameDecl *TD = D->getTypedefNameForAnonDecl()) {
Buf[off] = 'A';
Out << '@' << *TD;
-}
- else {
-if (D->isEmbeddedInDeclarator() && !D->isFreeStanding()) {
- printLoc(Out, D->getLocation(), Context->getSourceManager(), true);
} else {
- Buf[off] = 'a';
- if (auto *ED = dyn_cast(D)) {
-// Distinguish USRs of anonymous enums by using their first enumerator.
-auto enum_range = ED->enumerators();
-if (enum_range.begin() != enum_range.end()) {
- Out << '@' << **enum_range.begin();
+ if (D->isEmbeddedInDeclarator() && !D->isFreeStanding()) {
+printLoc(Out, D->getLocation(), Context->getSourceManager(), true);
+ } else {
+Buf[off] = 'a';
+if (auto *ED = dyn_cast(D)) {
+ // Distinguish USRs of anonymous enums by using their first
+ // enumerator.
+ auto enum_range = ED->enumerators();
+ if (enum_range.begin() != enum_range.end()) {
+Out << '@' << **enum_range.begin();
+ }
}
}
}
}
- }
// For a class template specialization, mangle the template arguments.
if (const ClassTemplateSpecializationDecl *Spec
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] ae4dce8 - [clang][ABI] New C++20 module mangling scheme
Author: Nathan Sidwell
Date: 2022-03-30T08:51:27-07:00
New Revision: ae4dce8659f313ca2034782583d31993212fa8bd
URL:
https://github.com/llvm/llvm-project/commit/ae4dce8659f313ca2034782583d31993212fa8bd
DIFF:
https://github.com/llvm/llvm-project/commit/ae4dce8659f313ca2034782583d31993212fa8bd.diff
LOG: [clang][ABI] New C++20 module mangling scheme
Implement a demangleable strong ownership symbol mangling.
* The original module symbol mangling scheme turned out to be
undemangleable.
* The hoped-for C++17 compatibility of weak ownership turns out to be
fragile
* C++20 now has better ways of controlling C++17 compatibility
The issue is captured on the ABI list at:
https://github.com/itanium-cxx-abi/cxx-abi/issues/134
GCC implements this new mangling.
The old mangling is unceremoniously dropped. No backwards
compatibility, no deprectated old-mangling flag. It was always
labelled experimental. (Old and new manglings cannot be confused.)
Reviewed By: dblaikie
Differential Revision: https://reviews.llvm.org/D122256
Added:
clang/test/CodeGenCXX/Inputs/cxx20-module-impl-1a.cpp
clang/test/CodeGenCXX/Inputs/cxx20-module-std-subst-2a.cpp
clang/test/CodeGenCXX/cxx20-module-decomp-1.cpp
clang/test/CodeGenCXX/cxx20-module-extern-1.cppm
clang/test/CodeGenCXX/cxx20-module-impl-1a.cpp
clang/test/CodeGenCXX/cxx20-module-nested-1.cppm
clang/test/CodeGenCXX/cxx20-module-nested-2.cppm
clang/test/CodeGenCXX/cxx20-module-part-1a.cpp
clang/test/CodeGenCXX/cxx20-module-part-1b.cpp
clang/test/CodeGenCXX/cxx20-module-part-1c.cpp
clang/test/CodeGenCXX/cxx20-module-std-subst-1.cppm
clang/test/CodeGenCXX/cxx20-module-std-subst-2b.cpp
clang/test/CodeGenCXX/cxx20-module-std-subst-2c.cpp
clang/test/CodeGenCXX/cxx20-module-sub-1a.cppm
clang/test/CodeGenCXX/cxx20-module-sub-1b.cppm
clang/test/CodeGenCXX/cxx20-module-tmpl-1.cppm
Modified:
clang/lib/AST/Decl.cpp
clang/lib/AST/ItaniumMangle.cpp
clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp
clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cppm
clang/test/CXX/modules-ts/basic/basic.def.odr/p4/user.cpp
clang/test/CXX/modules-ts/basic/basic.link/p3.cppm
clang/test/CXX/modules-ts/codegen-basics.cppm
Removed:
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 9b8585bbe3128..46c888430bb07 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -1547,6 +1547,11 @@ LinkageInfo
LinkageComputer::getDeclLinkageAndVisibility(const NamedDecl *D) {
}
Module *Decl::getOwningModuleForLinkage(bool IgnoreLinkage) const {
+ if (isa(this))
+// Namespaces never have module linkage. It is the entities within them
+// that [may] do.
+return nullptr;
+
Module *M = getOwningModule();
if (!M)
return nullptr;
diff --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index 96716f6ce0e31..fb76fa7b896fc 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -438,6 +438,7 @@ class CXXNameMangler {
void mangleType(QualType T);
void mangleNameOrStandardSubstitution(const NamedDecl *ND);
void mangleLambdaSig(const CXXRecordDecl *Lambda);
+ void mangleModuleNamePrefix(StringRef Name);
private:
@@ -473,22 +474,21 @@ class CXXNameMangler {
void mangleNameWithAbiTags(GlobalDecl GD,
const AbiTagList *AdditionalAbiTags);
- void mangleModuleName(const Module *M);
- void mangleModuleNamePrefix(StringRef Name);
+ void mangleModuleName(const NamedDecl *ND);
void mangleTemplateName(const TemplateDecl *TD,
const TemplateArgument *TemplateArgs,
unsigned NumTemplateArgs);
- void mangleUnqualifiedName(GlobalDecl GD,
+ void mangleUnqualifiedName(GlobalDecl GD, const DeclContext *DC,
const AbiTagList *AdditionalAbiTags) {
-mangleUnqualifiedName(GD, cast(GD.getDecl())->getDeclName(),
UnknownArity,
- AdditionalAbiTags);
+mangleUnqualifiedName(GD, cast(GD.getDecl())->getDeclName(), DC,
+ UnknownArity, AdditionalAbiTags);
}
void mangleUnqualifiedName(GlobalDecl GD, DeclarationName Name,
- unsigned KnownArity,
+ const DeclContext *DC, unsigned KnownArity,
const AbiTagList *AdditionalAbiTags);
- void mangleUnscopedName(GlobalDecl GD,
+ void mangleUnscopedName(GlobalDecl GD, const DeclContext *DC,
const AbiTagList *AdditionalAbiTags);
- void mangleUnscopedTemplateName(GlobalDecl GD,
+ void mangleUnscopedTemplateName(GlobalDecl GD, const DeclContext *DC,
const AbiTagList *AdditionalAbiTags);
void mangleSourceName(const IdentifierInfo *II);
void mangleRegCallName(c
[clang] 54c5033 - [clang] Document p1703 not needed
Author: Nathan Sidwell Date: 2022-04-05T06:54:31-07:00 New Revision: 54c50336e4c11e9c9255d96ae0b55a56cc193ecb URL: https://github.com/llvm/llvm-project/commit/54c50336e4c11e9c9255d96ae0b55a56cc193ecb DIFF: https://github.com/llvm/llvm-project/commit/54c50336e4c11e9c9255d96ae0b55a56cc193ecb.diff LOG: [clang] Document p1703 not needed We list p1703 as unimplemented, but it is subsumed by p1857. Reviewed By: tbaeder Differential Revision: https://reviews.llvm.org/D123120 Added: Modified: clang/www/cxx_status.html Removed: diff --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html index 3fcf7a3a27cae..47a6fdb1e4466 100755 --- a/clang/www/cxx_status.html +++ b/clang/www/cxx_status.html @@ -1176,10 +1176,11 @@ C++20 implementation status https://wg21.link/p1811r0";>P1811R0 -No +No https://wg21.link/p1703r1";>P1703R1 +Subsumed by P1857 https://wg21.link/p1874r1";>P1874R1 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] ba4482f - [clang][NFC] Add specificity to compatibility hack
Author: Nathan Sidwell
Date: 2022-04-06T03:57:36-07:00
New Revision: ba4482f481991799a922d5cf124a56bea5866254
URL:
https://github.com/llvm/llvm-project/commit/ba4482f481991799a922d5cf124a56bea5866254
DIFF:
https://github.com/llvm/llvm-project/commit/ba4482f481991799a922d5cf124a56bea5866254.diff
LOG: [clang][NFC] Add specificity to compatibility hack
Add specific dates and versions to note about source_location handling.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D123119
Added:
Modified:
clang/lib/AST/ExprConstant.cpp
Removed:
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 6c2447dc4420f..93950ac5341ba 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -8838,10 +8838,11 @@ bool PointerExprEvaluator::VisitCastExpr(const CastExpr
*E) {
E->getType()->getPointeeType());
// 1. We'll allow it in std::allocator::allocate, and anything which that
//calls.
- // 2. We'll allow it in the body of std::source_location:current. This
is
- //necessary for libstdc++'s , which gave its
- //parameter the type void*, and cast from that back to `const
__impl*`
- //in the body. (Fixed for new versions in gcc.gnu.org/PR104602).
+ // 2. HACK 2022-03-28: Work around an issue with libstdc++'s
+ // header. Fixed in GCC 12 and later (2022-04-??).
+ //We'll allow it in the body of std::source_location::current. GCC's
+ //implementation had a parameter of type `void*`, and casts from
+ //that back to `const __impl*` in its body.
if (VoidPtrCastMaybeOK &&
(Info.getStdAllocatorCaller("allocate") ||
IsDeclSourceLocationCurrent(Info.CurrentCall->Callee))) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 482fad4 - [clang][DOC] Document module mangler changes
Author: Nathan Sidwell Date: 2022-04-06T11:13:32-07:00 New Revision: 482fad4a3fcb013aa19e97661e3d66583d026bb8 URL: https://github.com/llvm/llvm-project/commit/482fad4a3fcb013aa19e97661e3d66583d026bb8 DIFF: https://github.com/llvm/llvm-project/commit/482fad4a3fcb013aa19e97661e3d66583d026bb8.diff LOG: [clang][DOC] Document module mangler changes Note that the mangling has changed and the demangler's learnt a new trick. Obviously dependent upon the mangler and demangler patches. Reviewed By: bruno Differential Revision: https://reviews.llvm.org/D123141 Added: Modified: clang/docs/ReleaseNotes.rst Removed: diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index bf592329ac02a..df6a7d7539ee6 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -201,6 +201,13 @@ C++20 Feature Support - Implemented `__builtin_source_location()` which enables library support for std::source_location. +- The mangling scheme for C++20 modules has incompatibly changed. The + initial mangling was discovered not to be reversible, and the weak + ownership design decision did not give the backwards compatibility + that was hoped for. C++20 since added ``extern "C++"`` semantics + that can be used for such compatibility. The demangler now demangles + symbols with named module attachment. + C++2b Feature Support ^ ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 9eda5fc - [clang] Verify internal entity module mangling
Author: Nathan Sidwell
Date: 2022-04-07T05:40:57-07:00
New Revision: 9eda5fc0c6eafd772d0e6ff015352136c5e505a4
URL:
https://github.com/llvm/llvm-project/commit/9eda5fc0c6eafd772d0e6ff015352136c5e505a4
DIFF:
https://github.com/llvm/llvm-project/commit/9eda5fc0c6eafd772d0e6ff015352136c5e505a4.diff
LOG: [clang] Verify internal entity module mangling
Internal symbol mangling is implementation-defined. We do not mangle
any module attachment, and this adds a test to verify that.
Reviewed By: dblaikie
Differential Revision: https://reviews.llvm.org/D123220
Added:
clang/test/CodeGenCXX/cxx20-module-internal.cppm
Modified:
Removed:
diff --git a/clang/test/CodeGenCXX/cxx20-module-internal.cppm
b/clang/test/CodeGenCXX/cxx20-module-internal.cppm
new file mode 100644
index 0..b45358325c423
--- /dev/null
+++ b/clang/test/CodeGenCXX/cxx20-module-internal.cppm
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -std=c++20 %s -triple %itanium_abi_triple -emit-llvm -o - |
FileCheck %s
+
+// internal-linkage symbol mangling is implementation defined. Let's
+// not mangle in the module attachment -- that unnecessarily bloats
+// the symbols.
+
+export module A;
+
+// CHECK-DAG: void @_ZL6addonev(
+static void addone() {}
+// CHECK-DAG: @_ZL1x =
+static int x = 5;
+
+namespace {
+// CHECK-DAG: void @_ZN12_GLOBAL__N_14frobEv(
+void frob() {}
+// CHECK-DAG: @_ZN12_GLOBAL__N_11yE =
+int y = 2;
+struct Bill {
+ void F();
+};
+// CHECK-DAG: void @_ZN12_GLOBAL__N_14Bill1FEv(
+void Bill::F() {}
+} // namespace
+
+// CHECK-DAG: void @_ZL4FrobPN12_GLOBAL__N_14BillE(
+static void Frob(Bill *b) {
+ if (b)
+b->F();
+}
+
+namespace N {
+// CHECK-DAG: void @_ZN1NL5innerEv(
+static void inner() {}
+// CHECK-DAG: @_ZN1NL1zE
+static int z = 3;
+} // namespace N
+
+// CHECK-DAG: void @_ZW1A6addsixv(
+void addsix() {
+ Frob(nullptr);
+ frob();
+ addone();
+ void(x + y + N::z);
+ N::inner();
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 815446c - [clang][NFC] Standard substitution checking cleanup
Author: Nathan Sidwell
Date: 2022-02-10T04:44:48-08:00
New Revision: 815446cd3e164b8be010864648d4104ad178b129
URL:
https://github.com/llvm/llvm-project/commit/815446cd3e164b8be010864648d4104ad178b129
DIFF:
https://github.com/llvm/llvm-project/commit/815446cd3e164b8be010864648d4104ad178b129.diff
LOG: [clang][NFC] Standard substitution checking cleanup
In preparing for module mangling changes I noticed some issues with
the way we check for std::basic_string instantiations and friends.
*) there's a single routine for std::basic_{i,o,io}stream but it is
templatized on the length of the name. Really? just use a
StringRef, rather than clone the entire routine just for
'basic_iostream'.
*) We have a helper routine to check for char type, and call it from
several places. But given all the instantiations are of the form
TPL ...> we could just check the first arg is char
and the later templated args are instantiating that same type. A
simpler type comparison.
*) Because basic_string has a third allocator parameter, it is open
coded, which I found a little confusing. But otherwise it's exactly
the same pattern as the iostream ones. Just tell that checker about
whether there's an expected allocator argument.[*]
*) We may as well return in each block of mangleStandardSubstitution
once we determine it is not one of the entities of interest -- it
certainly cannot be one of the other kinds of entities.
FWIW this shaves about 500 bytes off the executable.
[*] I suppose we could also have this routine a tri-value, with one to
indicat 'it is this name, but it's not the one you're looking for', to
avoid later calls trying different names?
Reviewd By: ChuanqiXu
Differential Revision: https://reviews.llvm.org/D119333
Added:
Modified:
clang/lib/AST/ItaniumMangle.cpp
Removed:
diff --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index 2e734e2b28cdb..b15669d426bd6 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -5969,27 +5969,19 @@ bool CXXNameMangler::mangleSubstitution(uintptr_t Ptr) {
return true;
}
-static bool isCharType(QualType T) {
- if (T.isNull())
+/// Returns whether S is a template specialization of std::Name with a single
+/// argument of type A.
+static bool isSpecializedAs(QualType S, llvm::StringRef Name, QualType A) {
+ if (S.isNull())
return false;
- return T->isSpecificBuiltinType(BuiltinType::Char_S) ||
-T->isSpecificBuiltinType(BuiltinType::Char_U);
-}
-
-/// Returns whether a given type is a template specialization of a given name
-/// with a single argument of type char.
-static bool isCharSpecialization(QualType T, const char *Name) {
- if (T.isNull())
-return false;
-
- const RecordType *RT = T->getAs();
+ const RecordType *RT = S->getAs();
if (!RT)
return false;
const ClassTemplateSpecializationDecl *SD =
dyn_cast(RT->getDecl());
- if (!SD)
+ if (!SD || !SD->getIdentifier()->isStr(Name))
return false;
if (!isStdNamespace(getEffectiveDeclContext(SD)))
@@ -5999,26 +5991,37 @@ static bool isCharSpecialization(QualType T, const char
*Name) {
if (TemplateArgs.size() != 1)
return false;
- if (!isCharType(TemplateArgs[0].getAsType()))
+ if (TemplateArgs[0].getAsType() != A)
return false;
- return SD->getIdentifier()->getName() == Name;
+ return true;
}
-template
-static bool isStreamCharSpecialization(const
ClassTemplateSpecializationDecl*SD,
- const char (&Str)[StrLen]) {
- if (!SD->getIdentifier()->isStr(Str))
+/// Returns whether SD is a template specialization std::Name [, std::allocator]>
+/// HasAllocator controls whether the 3rd template argument is needed.
+static bool isStdCharSpecialization(const ClassTemplateSpecializationDecl *SD,
+llvm::StringRef Name, bool HasAllocator) {
+ if (!SD->getIdentifier()->isStr(Name))
return false;
const TemplateArgumentList &TemplateArgs = SD->getTemplateArgs();
- if (TemplateArgs.size() != 2)
+ if (TemplateArgs.size() != (HasAllocator ? 3 : 2))
return false;
- if (!isCharType(TemplateArgs[0].getAsType()))
+ QualType A = TemplateArgs[0].getAsType();
+ if (A.isNull())
+return false;
+ // Plain 'char' is named Char_S or Char_U depending on the target ABI.
+ if (!A->isSpecificBuiltinType(BuiltinType::Char_S) &&
+ !A->isSpecificBuiltinType(BuiltinType::Char_U))
return false;
- if (!isCharSpecialization(TemplateArgs[1].getAsType(), "char_traits"))
+ if (!isSpecializedAs(TemplateArgs[1].getAsType(), "char_traits", A))
+return false;
+
+ if (HasAllocator &&
+ !isSpecializedAs(TemplateArgs[2].getAsType(), "allocator", A))
return false;
return true;
@@ -6031,6 +6034,7 @@ bool CXXNameMangler::mangleStandardSubstitution(const
NamedDecl *ND) {
Out << "St";
[clang] 0209390 - [clang][NFC] Remove IgnoreLinkageSpecDecls
Author: Nathan Sidwell
Date: 2022-02-15T04:28:45-08:00
New Revision: 02093906fa0fd5bacc61b2189ea643c78cd02509
URL:
https://github.com/llvm/llvm-project/commit/02093906fa0fd5bacc61b2189ea643c78cd02509
DIFF:
https://github.com/llvm/llvm-project/commit/02093906fa0fd5bacc61b2189ea643c78cd02509.diff
LOG: [clang][NFC] Remove IgnoreLinkageSpecDecls
The Itanium mangler uses IgnoreLinkageSpecDecls to strip linkage spec
contexts. It doesn't do this consistently, but there is no need for
it to do it at all. getEffectiveDeclContext never returns a linkage
spec, as it either recurses, uses getRedeclContext (which itself
removes the specs), or gets the decl context of non-namespace entities.
This patch removes the function and all calls to it. For safety I add
a couple of asserts to make sure we never get them.
Reviewed By: ChuanqiXu
Differential Revision: https://reviews.llvm.org/D119748
Added:
Modified:
clang/lib/AST/ItaniumMangle.cpp
Removed:
diff --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index b15669d426bd6..b92a6a07ff1f7 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -862,18 +862,9 @@ void CXXNameMangler::mangleFunctionEncodingBareType(const
FunctionDecl *FD) {
MangleReturnType, FD);
}
-static const DeclContext *IgnoreLinkageSpecDecls(const DeclContext *DC) {
- while (isa(DC)) {
-DC = getEffectiveParentContext(DC);
- }
-
- return DC;
-}
-
/// Return whether a given namespace is the 'std' namespace.
static bool isStd(const NamespaceDecl *NS) {
- if (!IgnoreLinkageSpecDecls(getEffectiveParentContext(NS))
-->isTranslationUnit())
+ if (!getEffectiveParentContext(NS)->isTranslationUnit())
return false;
const IdentifierInfo *II = NS->getOriginalNamespace()->getIdentifier();
@@ -978,7 +969,7 @@ void CXXNameMangler::mangleNameWithAbiTags(GlobalDecl GD,
return;
}
- DC = IgnoreLinkageSpecDecls(DC);
+ assert(!isa(DC) && "context cannot be LinkageSpecDecl");
if (isLocalContainerContext(DC)) {
mangleLocalName(GD, AdditionalAbiTags);
@@ -1054,7 +1045,7 @@ void CXXNameMangler::mangleModuleNamePrefix(StringRef
Name) {
void CXXNameMangler::mangleTemplateName(const TemplateDecl *TD,
const TemplateArgument *TemplateArgs,
unsigned NumTemplateArgs) {
- const DeclContext *DC = IgnoreLinkageSpecDecls(getEffectiveDeclContext(TD));
+ const DeclContext *DC = getEffectiveDeclContext(TD);
if (DC->isTranslationUnit() || isStdNamespace(DC)) {
mangleUnscopedTemplateName(TD, nullptr);
@@ -1070,7 +1061,7 @@ void CXXNameMangler::mangleUnscopedName(GlobalDecl GD,
// ::=
// ::= St# ::std::
- if (isStdNamespace(IgnoreLinkageSpecDecls(getEffectiveDeclContext(ND
+ if (isStdNamespace(getEffectiveDeclContext(ND)))
Out << "St";
mangleUnqualifiedName(GD, AdditionalAbiTags);
@@ -2030,7 +2021,7 @@ void CXXNameMangler::manglePrefix(const DeclContext *DC,
bool NoFunction) {
// ::= # empty
// ::=
- DC = IgnoreLinkageSpecDecls(DC);
+ assert(!isa(DC) && "prefix cannot be LinkageSpecDecl");
if (DC->isTranslationUnit())
return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 082f328 - [clang] Itanium mangler constructors
Author: Nathan Sidwell
Date: 2022-02-16T04:30:47-08:00
New Revision: 082f328899be9ed8a38b04a4e52be936f4875495
URL:
https://github.com/llvm/llvm-project/commit/082f328899be9ed8a38b04a4e52be936f4875495
DIFF:
https://github.com/llvm/llvm-project/commit/082f328899be9ed8a38b04a4e52be936f4875495.diff
LOG: [clang] Itanium mangler constructors
The Itanium mangler constructors use both NSDMI and explicit member
construction for default values. This is confusing.
*) Use NSDMIs wherever possible
*) Use forwarding ctor for the nesting case with an
llvm::raw_null_ostream (and explicitly set NullOut flag in that ctor).
*) Copy the ModuleSubstitutions. This is a bug with no effect in the
current mangling, but not in the newer mangling.
Reviewed By: ChuanqiXu
Differential Revision: https://reviews.llvm.org/D119550
Added:
Modified:
clang/lib/AST/ItaniumMangle.cpp
Removed:
diff --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index b92a6a07ff1f7..63e40a0f30721 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -267,10 +267,10 @@ class CXXNameMangler {
/// that's not a template specialization; otherwise it's the pattern
/// for that specialization.
const NamedDecl *Structor;
- unsigned StructorType;
+ unsigned StructorType = 0;
/// The next substitution sequence number.
- unsigned SeqID;
+ unsigned SeqID = 0;
class FunctionTypeDepthState {
unsigned Bits;
@@ -430,32 +430,32 @@ class CXXNameMangler {
public:
CXXNameMangler(ItaniumMangleContextImpl &C, raw_ostream &Out_,
const NamedDecl *D = nullptr, bool NullOut_ = false)
-: Context(C), Out(Out_), NullOut(NullOut_), Structor(getStructor(D)),
- StructorType(0), SeqID(0), AbiTagsRoot(AbiTags) {
+ : Context(C), Out(Out_), NullOut(NullOut_), Structor(getStructor(D)),
+AbiTagsRoot(AbiTags) {
// These can't be mangled without a ctor type or dtor type.
assert(!D || (!isa(D) &&
!isa(D)));
}
CXXNameMangler(ItaniumMangleContextImpl &C, raw_ostream &Out_,
const CXXConstructorDecl *D, CXXCtorType Type)
-: Context(C), Out(Out_), Structor(getStructor(D)), StructorType(Type),
- SeqID(0), AbiTagsRoot(AbiTags) { }
+ : Context(C), Out(Out_), Structor(getStructor(D)), StructorType(Type),
+AbiTagsRoot(AbiTags) {}
CXXNameMangler(ItaniumMangleContextImpl &C, raw_ostream &Out_,
const CXXDestructorDecl *D, CXXDtorType Type)
-: Context(C), Out(Out_), Structor(getStructor(D)), StructorType(Type),
- SeqID(0), AbiTagsRoot(AbiTags) { }
+ : Context(C), Out(Out_), Structor(getStructor(D)), StructorType(Type),
+AbiTagsRoot(AbiTags) {}
CXXNameMangler(CXXNameMangler &Outer, raw_ostream &Out_)
- : Context(Outer.Context), Out(Out_), NullOut(false),
-Structor(Outer.Structor), StructorType(Outer.StructorType),
-SeqID(Outer.SeqID), FunctionTypeDepth(Outer.FunctionTypeDepth),
-AbiTagsRoot(AbiTags), Substitutions(Outer.Substitutions) {}
+ : Context(Outer.Context), Out(Out_), Structor(Outer.Structor),
+StructorType(Outer.StructorType), SeqID(Outer.SeqID),
+FunctionTypeDepth(Outer.FunctionTypeDepth), AbiTagsRoot(AbiTags),
+Substitutions(Outer.Substitutions),
+ModuleSubstitutions(Outer.ModuleSubstitutions) {}
CXXNameMangler(CXXNameMangler &Outer, llvm::raw_null_ostream &Out_)
- : Context(Outer.Context), Out(Out_), NullOut(true),
-Structor(Outer.Structor), StructorType(Outer.StructorType),
-SeqID(Outer.SeqID), FunctionTypeDepth(Outer.FunctionTypeDepth),
-AbiTagsRoot(AbiTags), Substitutions(Outer.Substitutions) {}
+ : CXXNameMangler(Outer, (raw_ostream &)Out_) {
+NullOut = true;
+ }
raw_ostream &getStream() { return Out; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 80bebbc - [clang][NFC] Cleanup some coroutine tests
Author: Nathan Sidwell
Date: 2022-05-16T05:15:12-07:00
New Revision: 80bebbc7cb77979ef9d229450b7ea84e3e9c6a5a
URL:
https://github.com/llvm/llvm-project/commit/80bebbc7cb77979ef9d229450b7ea84e3e9c6a5a
DIFF:
https://github.com/llvm/llvm-project/commit/80bebbc7cb77979ef9d229450b7ea84e3e9c6a5a.diff
LOG: [clang][NFC] Cleanup some coroutine tests
I noticed these two tests emit a warning about a missing
unhandled_exception. That's irrelevant to what is being tested, but
is unnecessary noise.
Reviewed By: dblaikie
Differential Revision: https://reviews.llvm.org/D125535
Added:
Modified:
clang/test/CodeGenCoroutines/coro-ret-void.cpp
clang/test/CoverageMapping/coroutine.cpp
Removed:
diff --git a/clang/test/CodeGenCoroutines/coro-ret-void.cpp
b/clang/test/CodeGenCoroutines/coro-ret-void.cpp
index 7bc1bcd958b7a..ae139afabc4d4 100644
--- a/clang/test/CodeGenCoroutines/coro-ret-void.cpp
+++ b/clang/test/CodeGenCoroutines/coro-ret-void.cpp
@@ -8,6 +8,7 @@ struct coro1 {
std::suspend_never initial_suspend();
std::suspend_never final_suspend() noexcept;
void return_void();
+void unhandled_exception() noexcept;
};
};
@@ -39,6 +40,7 @@ struct coro2 {
std::suspend_never initial_suspend();
std::suspend_never final_suspend() noexcept;
void return_value(int);
+void unhandled_exception() noexcept;
};
};
diff --git a/clang/test/CoverageMapping/coroutine.cpp
b/clang/test/CoverageMapping/coroutine.cpp
index c9de301f81757..da38acc442be2 100644
--- a/clang/test/CoverageMapping/coroutine.cpp
+++ b/clang/test/CoverageMapping/coroutine.cpp
@@ -30,6 +30,7 @@ struct std::coroutine_traits {
int get_return_object();
suspend_always initial_suspend();
suspend_always final_suspend() noexcept;
+void unhandled_exception() noexcept;
void return_value(int);
};
};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] a1dcfb7 - [clang] Module global init mangling
Author: Nathan Sidwell
Date: 2022-05-23T09:03:10-07:00
New Revision: a1dcfb75ea8c31dd39edb6bdab6f54cde81cad85
URL:
https://github.com/llvm/llvm-project/commit/a1dcfb75ea8c31dd39edb6bdab6f54cde81cad85
DIFF:
https://github.com/llvm/llvm-project/commit/a1dcfb75ea8c31dd39edb6bdab6f54cde81cad85.diff
LOG: [clang] Module global init mangling
C++20 modules require emission of an initializer function, which is
called by importers of the module. This implements the mangling for
that function. It is the one place the ABI exposes partition names in
symbols -- but fortunately only needed by other TUs of that same module.
Reviewed By: bruno
Differential Revision: https://reviews.llvm.org/D122741
Added:
Modified:
clang/include/clang/AST/Mangle.h
clang/lib/AST/ItaniumMangle.cpp
Removed:
diff --git a/clang/include/clang/AST/Mangle.h
b/clang/include/clang/AST/Mangle.h
index 9bca97a611d08..96cc8c90a8e83 100644
--- a/clang/include/clang/AST/Mangle.h
+++ b/clang/include/clang/AST/Mangle.h
@@ -199,6 +199,8 @@ class ItaniumMangleContext : public MangleContext {
virtual void mangleDynamicStermFinalizer(const VarDecl *D, raw_ostream &) =
0;
+ virtual void mangleModuleInitializer(const Module *Module, raw_ostream &) =
0;
+
// This has to live here, otherwise the CXXNameMangler won't have access to
// it.
virtual DiscriminatorOverrideTy getDiscriminatorOverride() const = 0;
diff --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index b380e02fc8f7d..a9416397c90d6 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -130,6 +130,8 @@ class ItaniumMangleContextImpl : public
ItaniumMangleContext {
void mangleLambdaSig(const CXXRecordDecl *Lambda, raw_ostream &) override;
+ void mangleModuleInitializer(const Module *Module, raw_ostream &) override;
+
bool getNextDiscriminator(const NamedDecl *ND, unsigned &disc) {
// Lambda closure types are already numbered.
if (isLambda(ND))
@@ -438,7 +440,7 @@ class CXXNameMangler {
void mangleType(QualType T);
void mangleNameOrStandardSubstitution(const NamedDecl *ND);
void mangleLambdaSig(const CXXRecordDecl *Lambda);
- void mangleModuleNamePrefix(StringRef Name);
+ void mangleModuleNamePrefix(StringRef Name, bool IsPartition = false);
private:
@@ -1057,8 +1059,8 @@ void CXXNameMangler::mangleModuleName(const NamedDecl
*ND) {
// ::=
// ::=
// ::= W
-// ::= W P # not (yet) needed
-void CXXNameMangler::mangleModuleNamePrefix(StringRef Name) {
+// ::= W P
+void CXXNameMangler::mangleModuleNamePrefix(StringRef Name, bool IsPartition) {
// ::= S _
auto It = ModuleSubstitutions.find(Name);
if (It != ModuleSubstitutions.end()) {
@@ -1072,10 +1074,14 @@ void CXXNameMangler::mangleModuleNamePrefix(StringRef
Name) {
auto Parts = Name.rsplit('.');
if (Parts.second.empty())
Parts.second = Parts.first;
- else
-mangleModuleNamePrefix(Parts.first);
+ else {
+mangleModuleNamePrefix(Parts.first, IsPartition);
+IsPartition = false;
+ }
Out << 'W';
+ if (IsPartition)
+Out << 'P';
Out << Parts.second.size() << Parts.second;
ModuleSubstitutions.insert({Name, SeqID++});
}
@@ -6533,6 +6539,21 @@ void ItaniumMangleContextImpl::mangleLambdaSig(const
CXXRecordDecl *Lambda,
Mangler.mangleLambdaSig(Lambda);
}
+void ItaniumMangleContextImpl::mangleModuleInitializer(const Module *M,
+ raw_ostream &Out) {
+ // ::= GI # module initializer function
+ CXXNameMangler Mangler(*this, Out);
+ Mangler.getStream() << "_ZGI";
+ Mangler.mangleModuleNamePrefix(M->getPrimaryModuleInterfaceName());
+ if (M->isModulePartition()) {
+// The partition needs including, as partitions can have them too.
+auto Partition = M->Name.find(':');
+Mangler.mangleModuleNamePrefix(
+StringRef(&M->Name[Partition + 1], M->Name.size() - Partition - 1),
+/*IsPartition*/ true);
+ }
+}
+
ItaniumMangleContext *ItaniumMangleContext::create(ASTContext &Context,
DiagnosticsEngine &Diags,
bool IsAux) {
___
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 = B
[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 = B
[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)
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)
@@ -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 bitfie
[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] 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] 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 &T) : 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
&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
&Opts,
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 &Features,
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 = &llvm::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 &Features,
SHA2 = 0;
AES = 0;
DSP = 0;
- Unaligned = 1;
+ HasUnalignedAccess = true;
SoftF
[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] [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] 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 == Fi
[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] [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] 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,
bitsToCharUnits(Context.getTargetInfo().getRegisterWidth())
[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] 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] 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] 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] [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] 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)
@@ -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 &CGRL);
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)
@@ -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 &CGRL);
+
+ // Compute the access unit following this one -- which might be a barrier at
+ // Limit.
+ BitFieldAccessUnit accumulateNextUnit(RecordDecl::field_iterator FieldEnd,
+CharUnits Limit,
+const CGRecordLowering &CGRL) 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 &Probe, 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 &CGRL) 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 &CGRL)
+: 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) {
+ uin
[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)
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][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 &Types, 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 {
retur
[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] 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 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 &Types, 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] [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] 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] 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)
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:
@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] Robustify open mp test (PR #69739)
https://github.com/urnathan created
https://github.com/llvm/llvm-project/pull/69739
If the source path contains 'alias' this would spuriously fail. Be more
specific about not wanting [no]alias annotations.
>From bb391aa466577f4187af6b284ee5107090778a03 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Fri, 20 Oct 2023 11:43:08 -0400
Subject: [PATCH] [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()
___
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 edited 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
