[clang] Remove source range from CXXOperatorCallExpr #147028
Open
+7
−8
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Haojian Wu (hokein) ChangesThis patch stops storing a source range in This change allows us to retain the optimization #141058 when switching to 64-bit source locations. Performance results: Full diff: https://github.com/llvm/llvm-project/pull/147028.diff 5 Files Affected:
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 477373f07f25d..a22c32241ac61 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -84,7 +84,7 @@ class CXXOperatorCallExpr final : public CallExpr {
friend class ASTStmtReader;
friend class ASTStmtWriter;
- SourceRange Range;
+ SourceLocation BeginLoc;
// CXXOperatorCallExpr has some trailing objects belonging
// to CallExpr. See CallExpr for the details.
@@ -158,9 +158,9 @@ class CXXOperatorCallExpr final : public CallExpr {
: getOperatorLoc();
}
- SourceLocation getBeginLoc() const { return Range.getBegin(); }
- SourceLocation getEndLoc() const { return Range.getEnd(); }
- SourceRange getSourceRange() const { return Range; }
+ SourceLocation getBeginLoc() const { return BeginLoc; }
+ SourceLocation getEndLoc() const { return getSourceRangeImpl().getEnd(); }
+ SourceRange getSourceRange() const { return getSourceRangeImpl(); }
static bool classof(const Stmt *T) {
return T->getStmtClass() == CXXOperatorCallExprClass;
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index 764e20a54dc89..5833a6405125d 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -600,7 +600,7 @@ CXXOperatorCallExpr::CXXOperatorCallExpr(OverloadedOperatorKind OpKind,
assert(
(CXXOperatorCallExprBits.OperatorKind == static_cast<unsigned>(OpKind)) &&
"OperatorKind overflow!");
- Range = getSourceRangeImpl();
+ BeginLoc = getSourceRangeImpl().getBegin();
}
CXXOperatorCallExpr::CXXOperatorCallExpr(unsigned NumArgs, bool HasFPFeatures,
diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp
index 8945407cf666e..0166e493bf03f 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -1733,7 +1733,7 @@ void ASTStmtReader::VisitMSDependentExistsStmt(MSDependentExistsStmt *S) {
void ASTStmtReader::VisitCXXOperatorCallExpr(CXXOperatorCallExpr *E) {
VisitCallExpr(E);
E->CXXOperatorCallExprBits.OperatorKind = Record.readInt();
- E->Range = Record.readSourceRange();
+ E->BeginLoc = Record.readSourceLocation();
}
void ASTStmtReader::VisitCXXRewrittenBinaryOperator(
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index 7f1b39c242e01..5d6f48f651847 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2934,7 +2934,6 @@ void ASTWriter::WriteDeclAbbrevs() {
// CXXOperatorCallExpr
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Operator Kind
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Source Location
- Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Source Location
CXXOperatorCallExprAbbrev = Stream.EmitAbbrev(std::move(Abv));
// Abbreviation for EXPR_CXX_MEMBER_CALL
diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp
index 87536be8c8d98..be9bad9e96cc1 100644
--- a/clang/lib/Serialization/ASTWriterStmt.cpp
+++ b/clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1702,7 +1702,7 @@ void ASTStmtWriter::VisitMSDependentExistsStmt(MSDependentExistsStmt *S) {
void ASTStmtWriter::VisitCXXOperatorCallExpr(CXXOperatorCallExpr *E) {
VisitCallExpr(E);
Record.push_back(E->getOperator());
- Record.AddSourceRange(E->Range);
+ Record.AddSourceLocation(E->BeginLoc);
if (!E->hasStoredFPFeatures() && !static_cast<bool>(E->getADLCallKind()))
AbbrevToUse = Writer.getCXXOperatorCallExprAbbrev();
|
|
@llvm/pr-subscribers-clang-modules Author: Haojian Wu (hokein) ChangesThis patch stops storing a source range in This change allows us to retain the optimization #141058 when switching to 64-bit source locations. Performance results: Full diff: https://github.com/llvm/llvm-project/pull/147028.diff 5 Files Affected:
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 477373f07f25d..a22c32241ac61 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -84,7 +84,7 @@ class CXXOperatorCallExpr final : public CallExpr {
friend class ASTStmtReader;
friend class ASTStmtWriter;
- SourceRange Range;
+ SourceLocation BeginLoc;
// CXXOperatorCallExpr has some trailing objects belonging
// to CallExpr. See CallExpr for the details.
@@ -158,9 +158,9 @@ class CXXOperatorCallExpr final : public CallExpr {
: getOperatorLoc();
}
- SourceLocation getBeginLoc() const { return Range.getBegin(); }
- SourceLocation getEndLoc() const { return Range.getEnd(); }
- SourceRange getSourceRange() const { return Range; }
+ SourceLocation getBeginLoc() const { return BeginLoc; }
+ SourceLocation getEndLoc() const { return getSourceRangeImpl().getEnd(); }
+ SourceRange getSourceRange() const { return getSourceRangeImpl(); }
static bool classof(const Stmt *T) {
return T->getStmtClass() == CXXOperatorCallExprClass;
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index 764e20a54dc89..5833a6405125d 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -600,7 +600,7 @@ CXXOperatorCallExpr::CXXOperatorCallExpr(OverloadedOperatorKind OpKind,
assert(
(CXXOperatorCallExprBits.OperatorKind == static_cast<unsigned>(OpKind)) &&
"OperatorKind overflow!");
- Range = getSourceRangeImpl();
+ BeginLoc = getSourceRangeImpl().getBegin();
}
CXXOperatorCallExpr::CXXOperatorCallExpr(unsigned NumArgs, bool HasFPFeatures,
diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp
index 8945407cf666e..0166e493bf03f 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -1733,7 +1733,7 @@ void ASTStmtReader::VisitMSDependentExistsStmt(MSDependentExistsStmt *S) {
void ASTStmtReader::VisitCXXOperatorCallExpr(CXXOperatorCallExpr *E) {
VisitCallExpr(E);
E->CXXOperatorCallExprBits.OperatorKind = Record.readInt();
- E->Range = Record.readSourceRange();
+ E->BeginLoc = Record.readSourceLocation();
}
void ASTStmtReader::VisitCXXRewrittenBinaryOperator(
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index 7f1b39c242e01..5d6f48f651847 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2934,7 +2934,6 @@ void ASTWriter::WriteDeclAbbrevs() {
// CXXOperatorCallExpr
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Operator Kind
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Source Location
- Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Source Location
CXXOperatorCallExprAbbrev = Stream.EmitAbbrev(std::move(Abv));
// Abbreviation for EXPR_CXX_MEMBER_CALL
diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp
index 87536be8c8d98..be9bad9e96cc1 100644
--- a/clang/lib/Serialization/ASTWriterStmt.cpp
+++ b/clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1702,7 +1702,7 @@ void ASTStmtWriter::VisitMSDependentExistsStmt(MSDependentExistsStmt *S) {
void ASTStmtWriter::VisitCXXOperatorCallExpr(CXXOperatorCallExpr *E) {
VisitCallExpr(E);
Record.push_back(E->getOperator());
- Record.AddSourceRange(E->Range);
+ Record.AddSourceLocation(E->BeginLoc);
if (!E->hasStoredFPFeatures() && !static_cast<bool>(E->getADLCallKind()))
AbbrevToUse = Writer.getCXXOperatorCallExprAbbrev();
|
cor3ntin
reviewed
Jul 4, 2025
| SourceLocation getEndLoc() const { return Range.getEnd(); } | ||
| SourceRange getSourceRange() const { return Range; } | ||
| SourceLocation getBeginLoc() const { return BeginLoc; } | ||
| SourceLocation getEndLoc() const { return getSourceRangeImpl().getEnd(); } |
There was a problem hiding this comment.
I think we could simplify getEndLoc() to be a trimmed-down version of getSourceRangeImpl
- for OO_Call/ OO_Subscript =>
getRParenLoc() - for 1 argument = >
getArg(0)->getEndLoc() - for OO_PlusPlus/OO_MinusMinus =>
getOperatorLoc() - for 2 argument:
getArg(1)->getEndLoc()
I am not sure about the current handling of OO_Arrow in getSourceRangeImpl
There was a problem hiding this comment.
We could do this, and apply the same approach to getBeginLocImpl as well. However, I’m not sure it’s a good idea to split getSourceRangeImpl across two places -- it makes the logic harder to follow and reason about.
I actually tried this approach in an earlier version of the patch, but didn’t observe any performance benefit. Personally, I prefer the current version, which keeps all the logic centralized in getSourceRangeImpl for better maintainability and clarity.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch stops storing a source range in
CXXOperatorCallExprand keeps only the begin location.This change allows us to retain the optimization #141058 when switching to 64-bit source locations.
Performance results:
https://llvm-compile-time-tracker.com/compare.php?from=0588e8188c647460b641b09467fe6b13a8d510d5&to=5958f83476a8b8ba97936f262396d3ff98fb1662&stat=instructions:u