-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[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
base: main
Are you sure you want to change the base?
Changes from 10 commits
381c221
16077ed
fb437b0
dc33c5a
21cf4b4
7472299
99d11a5
fd4aaf5
63a8b54
3c34d90
fd278cd
8826a4b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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 | ||||||
|
@@ -2901,34 +2907,43 @@ 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 | ||||||
// the begin source location, which has a significant impact on perf as | ||||||
// getBeginLoc is assumed to be cheap. | ||||||
// The layourt is as follow: | ||||||
// CallExpr | Begin | 4 bytes left | Trailing Objects | ||||||
// CXXMemberCallExpr | Trailing Objects | ||||||
// A bit in CallExprBitfields indicates if source locations are presents. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
protected: | ||||||
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: | ||||||
|
@@ -2975,14 +2990,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: | ||||||
|
@@ -3028,6 +3043,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.HasTrailingSourceLoc) { | ||||||
CallExprBits.HasTrailingSourceLoc = false; | ||||||
updateTrailingSourceLoc(); | ||||||
erichkeane marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} | ||||||
} | ||||||
|
||||||
bool isCoroElideSafe() const { return CallExprBits.IsCoroElideSafe; } | ||||||
void setCoroElideSafe(bool V = true) { CallExprBits.IsCoroElideSafe = V; } | ||||||
|
||||||
|
@@ -3187,9 +3215,46 @@ 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; | ||||||
SourceLocation getBeginLoc() const { | ||||||
if (CallExprBits.HasTrailingSourceLoc) { | ||||||
assert(CallExprBits.HasTrailingSourceLoc && "No trailing source loc"); | ||||||
static_assert(sizeof(CallExpr) <= | ||||||
OffsetToTrailingObjects + sizeof(SourceLocation)); | ||||||
return *reinterpret_cast<const SourceLocation *>( | ||||||
reinterpret_cast<const char *>(this + 1)); | ||||||
} | ||||||
|
||||||
if (usesMemberSyntax()) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 { return getRParenLoc(); } | ||||||
|
||||||
private: | ||||||
friend class ASTStmtReader; | ||||||
bool hasTrailingSourceLoc() const { | ||||||
return CallExprBits.HasTrailingSourceLoc; | ||||||
} | ||||||
|
||||||
void updateTrailingSourceLoc() { | ||||||
assert(!CallExprBits.HasTrailingSourceLoc && | ||||||
"Trailing source loc already set?"); | ||||||
assert(getStmtClass() == CallExprClass && | ||||||
"Calling setTrailingSourceLocs on a subclass of CallExpr"); | ||||||
static_assert(sizeof(CallExpr) <= | ||||||
OffsetToTrailingObjects + sizeof(SourceLocation)); | ||||||
|
||||||
SourceLocation *Locs = | ||||||
reinterpret_cast<SourceLocation *>(reinterpret_cast<char *>(this + 1)); | ||||||
new (Locs) SourceLocation(getBeginLoc()); | ||||||
CallExprBits.HasTrailingSourceLoc = 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; | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
There was a problem hiding this comment.
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
?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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