Skip to content

[Clang] Optimize some getBeginLoc implementations #141058

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
117 changes: 98 additions & 19 deletions clang/include/clang/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -1344,7 +1344,13 @@ class DeclRefExpr final

SourceLocation getLocation() const { return DeclRefExprBits.Loc; }
void setLocation(SourceLocation L) { DeclRefExprBits.Loc = L; }
SourceLocation getBeginLoc() const LLVM_READONLY;

SourceLocation getBeginLoc() const {
if (hasQualifier())
return getQualifierLoc().getBeginLoc();
return DeclRefExprBits.Loc;
}

SourceLocation getEndLoc() const LLVM_READONLY;

/// Determine whether this declaration reference was preceded by a
Expand Down Expand Up @@ -2901,34 +2907,44 @@ class CallExpr : public Expr {
//
// * An optional of type FPOptionsOverride.
//
// Note that we store the offset in bytes from the this pointer to the start
// of the trailing objects. It would be perfectly possible to compute it
// based on the dynamic kind of the CallExpr. However 1.) we have plenty of
// space in the bit-fields of Stmt. 2.) It was benchmarked to be faster to
// compute this once and then load the offset from the bit-fields of Stmt,
// instead of re-computing the offset each time the trailing objects are
// accessed.
// CallExpr subclasses are asssumed to be 32 bytes or less, and CallExpr
// itself is 24 bytes. To avoid having to recompute or store the offset of the
// trailing objects, we put it at 32 bytes (such that it is suitable for all
// subclasses) We use the 8 bytes gap left for instances of CallExpr to store
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works today, but we keep talking about moving to 64-bit source locations (and that's getting more important thanks to modules), what's the idea for in that situation? Grow the size of CallExpr?

Copy link
Contributor Author

@cor3ntin cor3ntin May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have 4 bytes left now. But yeah, if we need more space, we would need a different strategy

// the begin and end source locations. Caching the begin source location in
// particular as a significant impact on perf as getBeginLoc is assumed to be
// cheap.
// The layourt is as follow:
// CallExpr | Begin | End | Trailing Objects
// CXXMemberCallExpr | Trailing Objects
// A bit in CallExprBitfields indicates if source locations are presents.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// A bit in CallExprBitfields indicates if source locations are presents.
// A bit in CallExprBitfields indicates if source locations are present.


protected:
static constexpr unsigned offsetToTrailingObjects = 32;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
static constexpr unsigned offsetToTrailingObjects = 32;
static constexpr unsigned OffsetToTrailingObjects = 32;

template <typename T>
static constexpr unsigned
sizeToAllocateForCallExprSubclass(unsigned SizeOfTrailingObjects) {
static_assert(sizeof(T) <= CallExpr::offsetToTrailingObjects);
return SizeOfTrailingObjects + CallExpr::offsetToTrailingObjects;
}

private:
/// Return a pointer to the start of the trailing array of "Stmt *".
Stmt **getTrailingStmts() {
return reinterpret_cast<Stmt **>(reinterpret_cast<char *>(this) +
CallExprBits.OffsetToTrailingObjects);
offsetToTrailingObjects);
}
Stmt *const *getTrailingStmts() const {
return const_cast<CallExpr *>(this)->getTrailingStmts();
}

/// Map a statement class to the appropriate offset in bytes from the
/// this pointer to the trailing objects.
static unsigned offsetToTrailingObjects(StmtClass SC);

unsigned getSizeOfTrailingStmts() const {
return (1 + getNumPreArgs() + getNumArgs()) * sizeof(Stmt *);
}

size_t getOffsetOfTrailingFPFeatures() const {
assert(hasStoredFPFeatures());
return CallExprBits.OffsetToTrailingObjects + getSizeOfTrailingStmts();
return offsetToTrailingObjects + getSizeOfTrailingStmts();
}

public:
Expand Down Expand Up @@ -2975,14 +2991,14 @@ class CallExpr : public Expr {
FPOptionsOverride *getTrailingFPFeatures() {
assert(hasStoredFPFeatures());
return reinterpret_cast<FPOptionsOverride *>(
reinterpret_cast<char *>(this) + CallExprBits.OffsetToTrailingObjects +
reinterpret_cast<char *>(this) + offsetToTrailingObjects +
getSizeOfTrailingStmts());
}
const FPOptionsOverride *getTrailingFPFeatures() const {
assert(hasStoredFPFeatures());
return reinterpret_cast<const FPOptionsOverride *>(
reinterpret_cast<const char *>(this) +
CallExprBits.OffsetToTrailingObjects + getSizeOfTrailingStmts());
reinterpret_cast<const char *>(this) + offsetToTrailingObjects +
getSizeOfTrailingStmts());
}

public:
Expand Down Expand Up @@ -3028,6 +3044,19 @@ class CallExpr : public Expr {

bool hasStoredFPFeatures() const { return CallExprBits.HasFPFeatures; }

bool usesMemberSyntax() const {
return CallExprBits.ExplicitObjectMemFunUsingMemberSyntax;
}
void setUsesMemberSyntax(bool V = true) {
CallExprBits.ExplicitObjectMemFunUsingMemberSyntax = V;
// Because the source location may be different for explicit
// member, we reset the cached values.
if (CallExprBits.HasTrailingSourceLocs) {
CallExprBits.HasTrailingSourceLocs = false;
updateTrailingSourceLocs();
}
}

bool isCoroElideSafe() const { return CallExprBits.IsCoroElideSafe; }
void setCoroElideSafe(bool V = true) { CallExprBits.IsCoroElideSafe = V; }

Expand Down Expand Up @@ -3187,9 +3216,59 @@ class CallExpr : public Expr {
SourceLocation getRParenLoc() const { return RParenLoc; }
void setRParenLoc(SourceLocation L) { RParenLoc = L; }

SourceLocation getBeginLoc() const LLVM_READONLY;
SourceLocation getEndLoc() const LLVM_READONLY;
template <unsigned N> SourceLocation getTrailingSourceLoc() const {
static_assert(N <= 1);
assert(CallExprBits.HasTrailingSourceLocs && "No trailing source loc");
static_assert(sizeof(CallExpr) <=
offsetToTrailingObjects + 2 * sizeof(SourceLocation));
return *reinterpret_cast<const SourceLocation *>(
reinterpret_cast<const char *>(this) + sizeof(CallExpr) +
sizeof(SourceLocation) * N);
}

SourceLocation getBeginLoc() const {
if (CallExprBits.HasTrailingSourceLocs)
return getTrailingSourceLoc<0>();

if (usesMemberSyntax()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no curleys on single liners?

if (auto FirstArgLoc = getArg(0)->getBeginLoc(); FirstArgLoc.isValid()) {
return FirstArgLoc;
}
}
return getCallee()->getBeginLoc();
}

SourceLocation getEndLoc() const {
if (CallExprBits.HasTrailingSourceLocs)
return getTrailingSourceLoc<0>();

SourceLocation end = getRParenLoc();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
SourceLocation end = getRParenLoc();
SourceLocation End = getRParenLoc();

if (end.isInvalid() && getNumArgs() > 0 && getArg(getNumArgs() - 1))
end = getArg(getNumArgs() - 1)->getEndLoc();
return end;
}

private:
bool hasTrailingSourceLoc() const {
return CallExprBits.HasTrailingSourceLocs;
}

void updateTrailingSourceLocs() {
assert(!CallExprBits.HasTrailingSourceLocs &&
"Trailing source loc already set?");
assert(getStmtClass() == CallExprClass &&
"Calling setTrailingSourceLocs on a subclass of CallExpr");
static_assert(sizeof(CallExpr) <=
offsetToTrailingObjects + 2 * sizeof(SourceLocation));

SourceLocation *Locs = reinterpret_cast<SourceLocation *>(
reinterpret_cast<char *>(this) + sizeof(CallExpr));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reinterpret_cast<SourceLocation *>(this + 1) ?

new (Locs) SourceLocation(getBeginLoc());
new (Locs + 1) SourceLocation(getEndLoc());
CallExprBits.HasTrailingSourceLocs = true;
}

public:
/// Return true if this is a call to __assume() or __builtin_assume() with
/// a non-value-dependent constant parameter evaluating as false.
bool isBuiltinAssumeFalse(const ASTContext &Ctx) const;
Expand Down
16 changes: 11 additions & 5 deletions clang/include/clang/AST/NestedNameSpecifier.h
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,9 @@ class NestedNameSpecifierLoc {
/// For example, if this instance refers to a nested-name-specifier
/// \c \::std::vector<int>::, the returned source range would cover
/// from the initial '::' to the last '::'.
SourceRange getSourceRange() const LLVM_READONLY;
SourceRange getSourceRange() const LLVM_READONLY {
return SourceRange(getBeginLoc(), getEndLoc());
}

/// Retrieve the source range covering just the last part of
/// this nested-name-specifier, not including the prefix.
Expand All @@ -296,14 +298,18 @@ class NestedNameSpecifierLoc {
/// Retrieve the location of the beginning of this
/// nested-name-specifier.
SourceLocation getBeginLoc() const {
return getSourceRange().getBegin();
if (!Qualifier)
return SourceLocation();

NestedNameSpecifierLoc First = *this;
while (NestedNameSpecifierLoc Prefix = First.getPrefix())
First = Prefix;
return First.getLocalSourceRange().getBegin();
Comment on lines +301 to +307
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't see the differences of changes on these functions... Are they related?

}

/// Retrieve the location of the end of this
/// nested-name-specifier.
SourceLocation getEndLoc() const {
return getSourceRange().getEnd();
}
SourceLocation getEndLoc() const { return getLocalSourceRange().getEnd(); }

/// Retrieve the location of the beginning of this
/// component of the nested-name-specifier.
Expand Down
18 changes: 11 additions & 7 deletions clang/include/clang/AST/Stmt.h
Original file line number Diff line number Diff line change
Expand Up @@ -563,17 +563,21 @@ class alignas(void *) Stmt {
unsigned HasFPFeatures : 1;

/// True if the call expression is a must-elide call to a coroutine.
LLVM_PREFERRED_TYPE(bool)
unsigned IsCoroElideSafe : 1;

/// Padding used to align OffsetToTrailingObjects to a byte multiple.
unsigned : 24 - 4 - NumExprBits;
/// Tracks When CallExpr is used to represent an explicit object
/// member function, in order to adjust the begin location.
LLVM_PREFERRED_TYPE(bool)
unsigned ExplicitObjectMemFunUsingMemberSyntax : 1;

/// The offset in bytes from the this pointer to the start of the
/// trailing objects belonging to CallExpr. Intentionally byte sized
/// for faster access.
unsigned OffsetToTrailingObjects : 8;
/// Indicates that SourceLocations are cached as
/// Trailing objects. See the definition of CallExpr.
LLVM_PREFERRED_TYPE(bool)
unsigned HasTrailingSourceLocs : 1;
};
enum { NumCallExprBits = 32 };

enum { NumCallExprBits = 25 };

class MemberExprBitfields {
friend class ASTStmtReader;
Expand Down
Loading
Loading