Skip to content

[NFC] Explicitly control alignment of Identifiers #27414

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 38 additions & 17 deletions include/swift/AST/Identifier.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,26 @@ class Identifier {

const char *Pointer;

public:
enum : size_t {
NumLowBitsAvailable = 2,
RequiredAlignment = 1 << NumLowBitsAvailable,
SpareBitMask = ((intptr_t)1 << NumLowBitsAvailable) - 1
};

private:
/// Constructor, only accessible by ASTContext, which handles the uniquing.
explicit Identifier(const char *Ptr) : Pointer(Ptr) {}
explicit Identifier(const char *Ptr) : Pointer(Ptr) {
assert(((uintptr_t)Ptr & SpareBitMask) == 0
&& "Identifier pointer does not use any spare bits");
}

/// A type with the alignment expected of a valid \c Identifier::Pointer .
struct alignas(uint32_t) Aligner {};

static_assert(alignof(Aligner) >= RequiredAlignment,
"Identifier table will provide enough spare bits");

public:
explicit Identifier() : Pointer(nullptr) {}

Expand Down Expand Up @@ -153,12 +171,15 @@ class Identifier {
bool operator<(Identifier RHS) const { return Pointer < RHS.Pointer; }

static Identifier getEmptyKey() {
return Identifier((const char*)
llvm::DenseMapInfo<const void*>::getEmptyKey());
uintptr_t Val = static_cast<uintptr_t>(-1);
Val <<= NumLowBitsAvailable;
return Identifier((const char*)Val);
}

static Identifier getTombstoneKey() {
return Identifier((const char*)
llvm::DenseMapInfo<const void*>::getTombstoneKey());
uintptr_t Val = static_cast<uintptr_t>(-2);
Val <<= NumLowBitsAvailable;
return Identifier((const char*)Val);
}

private:
Expand Down Expand Up @@ -202,7 +223,7 @@ namespace llvm {
static inline swift::Identifier getFromVoidPointer(void *P) {
return swift::Identifier::getFromOpaquePointer(P);
}
enum { NumLowBitsAvailable = 2 };
enum { NumLowBitsAvailable = swift::Identifier::NumLowBitsAvailable };
};

} // end namespace llvm
Expand All @@ -221,15 +242,15 @@ class DeclBaseName {
};

private:
/// In a special DeclName represenenting a subscript, this opaque pointer
/// In a special DeclName representing a subscript, this opaque pointer
/// is used as the data of the base name identifier.
/// This is an implementation detail that should never leak outside of
/// DeclName.
static void *SubscriptIdentifierData;
static const Identifier::Aligner SubscriptIdentifierData;
/// As above, for special constructor DeclNames.
static void *ConstructorIdentifierData;
static const Identifier::Aligner ConstructorIdentifierData;
/// As above, for special destructor DeclNames.
static void *DestructorIdentifierData;
static const Identifier::Aligner DestructorIdentifierData;

Identifier Ident;

Expand All @@ -239,23 +260,23 @@ class DeclBaseName {
DeclBaseName(Identifier I) : Ident(I) {}

static DeclBaseName createSubscript() {
return DeclBaseName(Identifier((const char *)SubscriptIdentifierData));
return DeclBaseName(Identifier((const char *)&SubscriptIdentifierData));
}

static DeclBaseName createConstructor() {
return DeclBaseName(Identifier((const char *)ConstructorIdentifierData));
return DeclBaseName(Identifier((const char *)&ConstructorIdentifierData));
}

static DeclBaseName createDestructor() {
return DeclBaseName(Identifier((const char *)DestructorIdentifierData));
return DeclBaseName(Identifier((const char *)&DestructorIdentifierData));
}

Kind getKind() const {
if (Ident.get() == SubscriptIdentifierData) {
if (Ident.get() == (const char *)&SubscriptIdentifierData) {
return Kind::Subscript;
} else if (Ident.get() == ConstructorIdentifierData) {
} else if (Ident.get() == (const char *)&ConstructorIdentifierData) {
return Kind::Constructor;
} else if (Ident.get() == DestructorIdentifierData) {
} else if (Ident.get() == (const char *)&DestructorIdentifierData) {
return Kind::Destructor;
} else {
return Kind::Normal;
Expand Down Expand Up @@ -720,7 +741,7 @@ namespace llvm {
static inline swift::DeclName getFromVoidPointer(void *ptr) {
return swift::DeclName::getFromOpaqueValue(ptr);
}
enum { NumLowBitsAvailable = 0 };
enum { NumLowBitsAvailable = PointerLikeTypeTraits<swift::DeclBaseName>::NumLowBitsAvailable - 2 };
};

// DeclNames hash just like pointers.
Expand Down
6 changes: 4 additions & 2 deletions lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,8 @@ struct ASTContext::Implementation {

// FIXME: This is a StringMap rather than a StringSet because StringSet
// doesn't allow passing in a pre-existing allocator.
llvm::StringMap<char, llvm::BumpPtrAllocator&> IdentifierTable;
llvm::StringMap<Identifier::Aligner, llvm::BumpPtrAllocator&>
IdentifierTable;

/// The declaration of Swift.AssignmentPrecedence.
PrecedenceGroupDecl *AssignmentPrecedence = nullptr;
Expand Down Expand Up @@ -642,7 +643,8 @@ Identifier ASTContext::getIdentifier(StringRef Str) const {
if (Str.data() == nullptr)
return Identifier(nullptr);

auto I = getImpl().IdentifierTable.insert(std::make_pair(Str, char())).first;
auto pair = std::make_pair(Str, Identifier::Aligner());
auto I = getImpl().IdentifierTable.insert(pair).first;
return Identifier(I->getKeyData());
}

Expand Down
9 changes: 3 additions & 6 deletions lib/AST/Identifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,9 @@
#include "clang/Basic/CharInfo.h"
using namespace swift;

void *DeclBaseName::SubscriptIdentifierData =
&DeclBaseName::SubscriptIdentifierData;
void *DeclBaseName::ConstructorIdentifierData =
&DeclBaseName::ConstructorIdentifierData;
void *DeclBaseName::DestructorIdentifierData =
&DeclBaseName::DestructorIdentifierData;
constexpr const Identifier::Aligner DeclBaseName::SubscriptIdentifierData{};
constexpr const Identifier::Aligner DeclBaseName::ConstructorIdentifierData{};
constexpr const Identifier::Aligner DeclBaseName::DestructorIdentifierData{};

raw_ostream &llvm::operator<<(raw_ostream &OS, Identifier I) {
if (I.get() == nullptr)
Expand Down