[clang] Optimize SourceManager.getSpellingLocSlowCase and SourceManager.getFileLocSlowCase #164269

SergejSalnikov · 2025-10-20T15:18:48Z

Optimize implementations of getSpellingLocSlowCase and getFileLocSlowCase by inlining called methods to avoid repeated calls to getSLocEntry and getFileID.

a performance improvement follow up for #160667

Optimize implementations of `getSpellingLocSlowCase` and `getFileLocSlowCase` by inlining methods to avoid repeated calls to `getSLocEntry`

github-actions · 2025-10-20T15:19:08Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2025-10-20T15:19:51Z

@llvm/pr-subscribers-clang

Author: SKill (SergejSalnikov)

Changes

Optimize implementations of getSpellingLocSlowCase and getFileLocSlowCase by inlining methods to avoid repeated calls to getSLocEntry and getFileIDSlow.

Full diff: https://github.com/llvm/llvm-project/pull/164269.diff

1 Files Affected:

(modified) clang/lib/Basic/SourceManager.cpp (+11-7)

diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp
index d8ec837f0f7b9..cedbd31dcef72 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -908,19 +908,23 @@ getExpansionLocSlowCase(SourceLocation Loc) const {
 
 SourceLocation SourceManager::getSpellingLocSlowCase(SourceLocation Loc) const {
   do {
-    FileIDAndOffset LocInfo = getDecomposedLoc(Loc);
-    Loc = getSLocEntry(LocInfo.first).getExpansion().getSpellingLoc();
-    Loc = Loc.getLocWithOffset(LocInfo.second);
+    const SLocEntry &Entry = getSLocEntry(getFileID(Loc));
+    Loc = Entry.getExpansion().getSpellingLoc().getLocWithOffset(
+        Loc.getOffset() - Entry.getOffset());
   } while (!Loc.isFileID());
   return Loc;
 }
 
 SourceLocation SourceManager::getFileLocSlowCase(SourceLocation Loc) const {
   do {
-    if (isMacroArgExpansion(Loc))
-      Loc = getImmediateSpellingLoc(Loc);
-    else
-      Loc = getImmediateExpansionRange(Loc).getBegin();
+    const SLocEntry &Entry = getSLocEntry(getFileID(Loc));
+    const ExpansionInfo &ExpInfo = Entry.getExpansion();
+    if (ExpInfo.isMacroArgExpansion()) {
+      Loc = ExpInfo.getSpellingLoc().getLocWithOffset(Loc.getOffset() -
+                                                      Entry.getOffset());
+    } else {
+      Loc = ExpInfo.getExpansionLocStart();
+    }
   } while (!Loc.isFileID());
   return Loc;
 }

github-actions · 2025-10-20T15:25:22Z

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Developer Policy and LLVM Discourse for more information.

SergejSalnikov · 2025-10-20T17:17:18Z

@zwuis, Looks like unrelated failure.

AaronBallman · 2025-10-24T15:17:31Z

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo. Please turn off Keep my email addresses private setting in your account. See LLVM Developer Policy and LLVM Discourse for more information.

FYI

AaronBallman · 2025-10-24T15:18:16Z

Optimize implementations of getSpellingLocSlowCase and getFileLocSlowCase by inlining called methods to avoid repeated calls to getSLocEntry and getFileID.

#160667

I think the patch summary and title should be updated, this isn't so much about optimization as it is fixing a bug with debug info, right?

SergejSalnikov · 2025-10-24T15:21:11Z

This PR is functionally no-op. It's purely code optimisation to reduce the number of back-to-back getSLocEntry and getFileID calls.

AaronBallman · 2025-10-24T15:22:04Z

This PR is functionally no-op. It's purely code optimisation to reduce the number of back-to-back getSLocEntry and getFileID calls.

Oh yeah, I see #163982 is the PR for the debug info changes, that makes more sense. :-)

AaronBallman · 2025-10-24T15:22:24Z

clang/lib/Basic/SourceManager.cpp

+    const SLocEntry &Entry = getSLocEntry(getFileID(Loc));
+    const ExpansionInfo &ExpInfo = Entry.getExpansion();
+    if (ExpInfo.isMacroArgExpansion()) {
+      Loc = ExpInfo.getSpellingLoc().getLocWithOffset(Loc.getOffset() -
+                                                      Entry.getOffset());
+    } else {
+      Loc = ExpInfo.getExpansionLocStart();
+    }


The changes look correct, but I guess I'm surprised there's any measurable impact on performance; are there performance measurement numbers for these changes? (We have https://llvm-compile-time-tracker.com/ to help measure that sort of thing, FWIW.)

Probably you are correct.
But it looks nicer that way. The performance depends how macro heavy is the source code. I bet you can save a few seconds on the large codebases.

I found the old code to be easier to understand what was going on, so I worry about premature optimization without some idea of measurements. CC @erichkeane @Sirraide @Endilll for some other opinions

Can we run the performance test on this RP somehow? Although I doubt that there will be any significant change.

Yeah, both of these cases are a little tougher to read for me. I'd like to see the perf measurements. I think @Endilll knows how to start them.

I believe that PR is more relevant in the context of #163982

Can we run the performance test on this RP somehow? Although I doubt that there will be any significant change.

Unfortunately, not trivially; you can contact Nikita to have your fork added (https://llvm-compile-time-tracker.com/about.php) but that may take a while given the upcoming dev conference and folks being busy with that. I think it'd be fine if there were some local tests on whatever project you have that's super macro heavy, something as simple as "average of 1234ms before this patch and 567ms after the patch on my machine" would perhaps help.

Basically, if there's a measurable performance improvement, we can hold our nose on slightly reduced readability, but if there's no measurable performance improvement, there's less of a case for taking the patch..

If that helps, here you can follow what LLVM compiler time tracker does.

I've tested the following 3 PRs together, but I don't know how to interpret the results.
#163982
#164269
#163982

full report

tbaederr · 2025-10-27T08:12:34Z

https://llvm-compile-time-tracker.com/compare.php?from=750a58337ee8594f5253733786e066f7941d617d&to=55e77ea80d4201544c151ac42bd154889f342d6c&stat=instructions:u

SergejSalnikov · 2025-10-31T09:54:13Z

@tbaederr, would you mind running performance tests again on this PR after beadb9e is merged?

tbaederr · 2025-10-31T14:23:17Z

https://llvm-compile-time-tracker.com/compare.php?from=9f9b8d56f47913496d1d7f80f6859437bde09fe7&to=ac9b64e81e47021304db618439f2bdc5d0ac33c3&stat=instructions:u

SergejSalnikov · 2025-10-31T15:00:04Z

Do we measure the compilation time on a mixed targets?

cor3ntin

This change seems fine to me, thanks!

SergejSalnikov · 2025-11-03T12:21:17Z

As there was some push back from other members of the team regarding readability of this CL. I'll let the decision on whether to merge this PR to the llvm team. Please merge the CL on my behalf if you think it's worth having it in.

AaronBallman

Thanks! The performance measurements seem to be a very slight improvement, and the code isn't significantly less readable, so I'd say let's land the changes.

github-actions · 2025-11-03T15:12:37Z

@SergejSalnikov Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

Optimize getSpellingLocSlowCase and getFileLocSlowCase

f27f647

Optimize implementations of `getSpellingLocSlowCase` and `getFileLocSlowCase` by inlining methods to avoid repeated calls to `getSLocEntry`

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 20, 2025

SergejSalnikov changed the title ~~Optimize getSpellingLocSlowCase and getFileLocSlowCase~~ [clang] Optimize SourceManager.getSpellingLocSlowCase and SourceManager.getFileLocSlowCase Oct 20, 2025

zwuis requested a review from AaronBallman October 21, 2025 00:32

dwblaikie mentioned this pull request Oct 23, 2025

[DebugInfo] Use locations inside macros (rather than the macro usage location) for instruction source locations in debug info #160667

Closed

AaronBallman reviewed Oct 24, 2025

View reviewed changes

Merge branch 'main' into optimize

8a1dc7a

cor3ntin approved these changes Nov 1, 2025

View reviewed changes

AaronBallman approved these changes Nov 3, 2025

View reviewed changes

AaronBallman merged commit 4f61863 into llvm:main Nov 3, 2025
10 checks passed

[clang] Optimize SourceManager.getSpellingLocSlowCase and SourceManager.getFileLocSlowCase #164269

[clang] Optimize SourceManager.getSpellingLocSlowCase and SourceManager.getFileLocSlowCase #164269

Uh oh!

Conversation

SergejSalnikov commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 20, 2025

Uh oh!

llvmbot commented Oct 20, 2025

Uh oh!

github-actions bot commented Oct 20, 2025

Uh oh!

SergejSalnikov commented Oct 20, 2025

Uh oh!

AaronBallman commented Oct 24, 2025

Uh oh!

AaronBallman commented Oct 24, 2025

Uh oh!

SergejSalnikov commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AaronBallman commented Oct 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SergejSalnikov Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tbaederr commented Oct 27, 2025

Uh oh!

SergejSalnikov commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tbaederr commented Oct 31, 2025

Uh oh!

SergejSalnikov commented Oct 31, 2025

Uh oh!

cor3ntin left a comment

Choose a reason for hiding this comment

Uh oh!

SergejSalnikov commented Nov 3, 2025

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Nov 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

SergejSalnikov commented Oct 20, 2025 •

edited

Loading

SergejSalnikov commented Oct 24, 2025 •

edited

Loading

SergejSalnikov Oct 25, 2025 •

edited

Loading

SergejSalnikov commented Oct 31, 2025 •

edited

Loading