Smaller funclet prologs/epilogs (x64) - #115284

Merged
jkotas merged 6 commits into
dotnet:mainfrom
jkotas:funclets
May 10, 2025
Merged

Smaller funclet prologs/epilogs (x64)#115284
jkotas merged 6 commits into
dotnet:mainfrom
jkotas:funclets

Conversation

@jkotas

@jkotasjkotas commented May 4, 2025

Copy link
Copy Markdown
Member
  • Allow funclets to trash non-volatile registers to make funclet prologs/epilogs smaller
  • Restore non-volatile registers before non-filter funclet calls
  • Unify funclet ABI between NAOT and non-NAOT

@jkotas
Smaller funclets prologs/epilogs (x64)
3d10555
- Allow funclets to trash non-volatile registers to make funclet prologs/epilogs smaller
- Restore non-volatile registers before non-filter funclet calls
- Unify funclet ABI between NAOT and non-NAOT
CopilotAI review requested due to automatic review settings May 4, 2025 18:57
@ghostghost added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 4, 2025
@jkotasjkotas added area-ExceptionHandling-coreclr only use for closed issues and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels May 4, 2025
CopilotAI reviewed May 4, 2025
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the funclet prolog/epilog sequences for x64 to reduce their size while unifying the ABI between NAOT and non-NAOT builds.

  • Removed redundant saving/restoring of non-volatile registers in funclets
  • Updated stack frame allocation and alignment computations
  • Adjusted documentation to reflect the revised behavior and design decisions

Reviewed Changes

Copilot reviewed 2 out of 6 changed files in this pull request and generated 2 comments.

FileDescription
src/coreclr/jit/codegenxarch.cppRemoved EBP and callee-register save/restore logic; updated frame allocation calculation using REG_NA
docs/design/coreclr/botr/clr-abi.mdRevised description of register preservation behavior in funclets
Files not reviewed (4)
  • src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.S: Language not supported
  • src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm: Language not supported
  • src/coreclr/vm/amd64/AsmHelpers.asm: Language not supported
  • src/coreclr/vm/amd64/asmhelpers.S: Language not supported

Comment threadsrc/coreclr/jit/codegenxarch.cpp Outdated
Comment threaddocs/design/coreclr/botr/clr-abi.md
@jkotasjkotas changed the title Smaller funclets prologs/epilogs (x64)Smaller funclet prologs/epilogs (x64)May 4, 2025
This was referenced May 4, 2025
Open
Open
Open
@jkotas

jkotas commented May 4, 2025

Copy link
Copy Markdown
MemberAuthor

@EgorBot -windows_intel -amd

usingSystem.Reflection;usingBenchmarkDotNet.Attributes;publicclassBench{staticintx;[Benchmark]publicvoidFinally(){try{try{try{try{try{try{try{try{try{try{thrownewException();}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}catch{}}}

@EgorBotEgorBot mentioned this pull request May 4, 2025
Open
@jkotas

jkotas commented May 4, 2025

Copy link
Copy Markdown
MemberAuthor

@EgorBot -windows_intel -intel

usingSystem.Reflection;usingBenchmarkDotNet.Attributes;publicclassBench{staticintx;[Benchmark]publicvoidFinally(){try{try{try{try{try{try{try{try{try{try{thrownewException();}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}catch{}}}

@EgorBotEgorBot mentioned this pull request May 4, 2025
Open
@jkotas

jkotas commented May 4, 2025

Copy link
Copy Markdown
MemberAuthor

@MihuBot

@MihuBotMihuBot mentioned this pull request May 4, 2025
Open
@jkotas

jkotas commented May 5, 2025•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Perf results summary:

  • 0.6% code size improvement on average
  • Artificial micro-benchmark to measure the cost of saving and restoring all registers around funclet invocation is very noisy. The results vary between 10% improvement and 5% regression.

@jkotas

jkotas commented May 5, 2025

Copy link
Copy Markdown
MemberAuthor

@janvorli @dotnet/jit-contrib PTLA. This PR is for x64 only. I plan to switch other platforms to the unified scheme in subsequent PRs.

janvorli
janvorli reviewed May 5, 2025
View reviewed changes
Comment threadsrc/coreclr/vm/amd64/AsmHelpers.asm
Comment threadsrc/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm Outdated
@build-analysisbuild-analysisBot mentioned this pull request May 5, 2025
Closed
janvorli
janvorli approved these changes May 5, 2025
View reviewed changes

@janvorlijanvorli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thank you!

BruceForstall
BruceForstall approved these changes May 5, 2025
View reviewed changes
Comment threadsrc/coreclr/jit/codegenxarch.cpp
@BruceForstall

BruceForstall commented May 5, 2025

Copy link
Copy Markdown
Contributor

I suggest running outerloop tests.

@jkotas

jkotas commented May 6, 2025

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr outerloop

@azure-pipelines

azure-pipelinesBot commented May 6, 2025

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@build-analysisbuild-analysisBot mentioned this pull request May 6, 2025
Open
3 tasks
@jkotas
jkotas merged commit 33b5215 into dotnet:mainMay 10, 2025
@jkotas
jkotas deleted the funclets branch May 10, 2025 05:41
@LoopedBard3LoopedBard3 mentioned this pull request May 13, 2025
Closed
@jkotasjkotas mentioned this pull request May 19, 2025
Closed
@LoopedBard3LoopedBard3 mentioned this pull request May 20, 2025
Closed
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jun 9, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@EgorBoEgorBoEgorBo left review comments
Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@janvorlijanvorlijanvorli approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovskyMichalStrehovsky is a code owner
+1 more reviewer
@BruceForstallBruceForstallBruceForstall approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jkotasjkotas

Labels

area-ExceptionHandling-coreclronly use for closed issues

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jkotas@BruceForstall@EgorBo@janvorli
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Smaller funclet prologs/epilogs (x64) - #115284
Merged
jkotas merged 6 commits into
dotnet:mainfrom 
jkotas:funclets
May 10, 2025
Merged
Smaller funclet prologs/epilogs (x64)#115284
jkotas merged 6 commits into
dotnet:mainfrom 
jkotas:funclets
Conversation
@jkotas
@jkotasjkotas
commented
May 4, 2025

Copy link


Copy Markdown

Member
  • Allow funclets to trash non-volatile registers to make funclet prologs/epilogs smaller
  • Restore non-volatile registers before non-filter funclet calls
  • Unify funclet ABI between NAOT and non-NAOT
@jkotas
Smaller funclets prologs/epilogs (x64)
3d10555
- Allow funclets to trash non-volatile registers to make funclet prologs/epilogs smaller
- Restore non-volatile registers before non-filter funclet calls
- Unify funclet ABI between NAOT and non-NAOT
CopilotAI
review requested due to automatic review settings
May 4, 2025 18:57
@ghostghost
added
the 
needs-area-label
An area label is needed to ensure this gets routed to the appropriate area owners
label
May 4, 2025
@jkotasjkotas
added

area-ExceptionHandling-coreclr
only use for closed issues
and removed

needs-area-label
An area label is needed to ensure this gets routed to the appropriate area owners
labels
May 4, 2025
CopilotAI
reviewed
May 4, 2025
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull Request Overview
This PR refactors the funclet prolog/epilog sequences for x64 to reduce their size while unifying the ABI between NAOT and non-NAOT builds.
  • Removed redundant saving/restoring of non-volatile registers in funclets
  • Updated stack frame allocation and alignment computations
  • Adjusted documentation to reflect the revised behavior and design decisions
Reviewed Changes
Copilot reviewed 2 out of 6 changed files in this pull request and generated 2 comments.
FileDescription
src/coreclr/jit/codegenxarch.cppRemoved EBP and callee-register save/restore logic; updated frame allocation calculation using REG_NA
docs/design/coreclr/botr/clr-abi.mdRevised description of register preservation behavior in funclets
Files not reviewed (4)
  • src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.S: Language not supported
  • src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm: Language not supported
  • src/coreclr/vm/amd64/AsmHelpers.asm: Language not supported
  • src/coreclr/vm/amd64/asmhelpers.S: Language not supported
Comment threadsrc/coreclr/jit/codegenxarch.cpp
Outdated

Comment threaddocs/design/coreclr/botr/clr-abi.md
@jkotasjkotas
changed the title
Smaller funclets prologs/epilogs (x64)Smaller funclet prologs/epilogs (x64)May 4, 2025

This was referenced May 4, 2025

Open


Open


Open

@jkotas
jkotas
commented
May 4, 2025

Copy link


Copy Markdown

MemberAuthor
@EgorBot -windows_intel -amd
usingSystem.Reflection;usingBenchmarkDotNet.Attributes;publicclassBench{staticintx;[Benchmark]publicvoidFinally(){try{try{try{try{try{try{try{try{try{try{thrownewException();}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}catch{}}}
@EgorBotEgorBot
mentioned this pull request
May 4, 2025

Open

@jkotas
jkotas
commented
May 4, 2025

Copy link


Copy Markdown

MemberAuthor
@EgorBot -windows_intel -intel
usingSystem.Reflection;usingBenchmarkDotNet.Attributes;publicclassBench{staticintx;[Benchmark]publicvoidFinally(){try{try{try{try{try{try{try{try{try{try{thrownewException();}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}catch{}}}
@EgorBotEgorBot
mentioned this pull request
May 4, 2025

Open

@jkotas
jkotas
commented
May 4, 2025

Copy link


Copy Markdown

MemberAuthor
@MihuBot
@MihuBotMihuBot
mentioned this pull request
May 4, 2025

Open

@jkotas
jkotas
commented
May 5, 2025•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
Perf results summary:
  • 0.6% code size improvement on average
  • Artificial micro-benchmark to measure the cost of saving and restoring all registers around funclet invocation is very noisy. The results vary between 10% improvement and 5% regression.
@jkotas
jkotas
commented
May 5, 2025

Copy link


Copy Markdown

MemberAuthor
@janvorli @dotnet/jit-contrib PTLA. This PR is for x64 only. I plan to switch other platforms to the unified scheme in subsequent PRs.
janvorli
janvorli
reviewed
May 5, 2025
View reviewed changes
Comment threadsrc/coreclr/vm/amd64/AsmHelpers.asm
Comment threadsrc/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm
Outdated

@build-analysisbuild-analysisBot
mentioned this pull request
May 5, 2025

Closed

janvorli
janvorli
approved these changes
May 5, 2025
View reviewed changes
@janvorlijanvorli
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

LGTM, thank you!
BruceForstall
BruceForstall
approved these changes
May 5, 2025
View reviewed changes
Comment threadsrc/coreclr/jit/codegenxarch.cpp
@BruceForstall
BruceForstall
commented
May 5, 2025

Copy link


Copy Markdown

Contributor
I suggest running outerloop tests.
@jkotas
jkotas
commented
May 6, 2025

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-coreclr outerloop
@azure-pipelines
azure-pipelinesBot
commented
May 6, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@build-analysisbuild-analysisBot
mentioned this pull request
May 6, 2025

Open

3 tasks
@jkotas
jkotas
merged commit 33b5215
into
dotnet:mainMay 10, 2025
@jkotas
jkotas
deleted the

funclets

branch
May 10, 2025 05:41
@LoopedBard3LoopedBard3
mentioned this pull request
May 13, 2025

Closed

@jkotasjkotas
mentioned this pull request
May 19, 2025

Closed

@LoopedBard3LoopedBard3
mentioned this pull request
May 20, 2025

Closed

@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jun 9, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@EgorBoEgorBoEgorBo left review comments
Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@janvorlijanvorlijanvorli approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovskyMichalStrehovsky is a code owner

+1 more reviewer

@BruceForstallBruceForstallBruceForstall approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jkotasjkotas

Labels

area-ExceptionHandling-coreclronly use for closed issues

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

5 participants

@jkotas@BruceForstall@EgorBo@janvorli

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Smaller funclet prologs/epilogs (x64) - #115284
Merged
jkotas merged 6 commits into
dotnet:mainfrom 
jkotas:funclets
May 10, 2025
Merged
Smaller funclet prologs/epilogs (x64)#115284
jkotas merged 6 commits into
dotnet:mainfrom 
jkotas:funclets
Conversation
@jkotas
@jkotasjkotas
commented
May 4, 2025

Copy link


Copy Markdown

Member
  • Allow funclets to trash non-volatile registers to make funclet prologs/epilogs smaller
  • Restore non-volatile registers before non-filter funclet calls
  • Unify funclet ABI between NAOT and non-NAOT
@jkotas
Smaller funclets prologs/epilogs (x64)
3d10555
- Allow funclets to trash non-volatile registers to make funclet prologs/epilogs smaller
- Restore non-volatile registers before non-filter funclet calls
- Unify funclet ABI between NAOT and non-NAOT
CopilotAI
review requested due to automatic review settings
May 4, 2025 18:57
@ghostghost
added
the 
needs-area-label
An area label is needed to ensure this gets routed to the appropriate area owners
label
May 4, 2025
@jkotasjkotas
added

area-ExceptionHandling-coreclr
only use for closed issues
and removed

needs-area-label
An area label is needed to ensure this gets routed to the appropriate area owners
labels
May 4, 2025
CopilotAI
reviewed
May 4, 2025
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull Request Overview
This PR refactors the funclet prolog/epilog sequences for x64 to reduce their size while unifying the ABI between NAOT and non-NAOT builds.
  • Removed redundant saving/restoring of non-volatile registers in funclets
  • Updated stack frame allocation and alignment computations
  • Adjusted documentation to reflect the revised behavior and design decisions
Reviewed Changes
Copilot reviewed 2 out of 6 changed files in this pull request and generated 2 comments.
FileDescription
src/coreclr/jit/codegenxarch.cppRemoved EBP and callee-register save/restore logic; updated frame allocation calculation using REG_NA
docs/design/coreclr/botr/clr-abi.mdRevised description of register preservation behavior in funclets
Files not reviewed (4)
  • src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.S: Language not supported
  • src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm: Language not supported
  • src/coreclr/vm/amd64/AsmHelpers.asm: Language not supported
  • src/coreclr/vm/amd64/asmhelpers.S: Language not supported
Comment threadsrc/coreclr/jit/codegenxarch.cpp
Outdated

Comment threaddocs/design/coreclr/botr/clr-abi.md
@jkotasjkotas
changed the title
Smaller funclets prologs/epilogs (x64)Smaller funclet prologs/epilogs (x64)May 4, 2025

This was referenced May 4, 2025

Open


Open


Open

@jkotas
jkotas
commented
May 4, 2025

Copy link


Copy Markdown

MemberAuthor
@EgorBot -windows_intel -amd
usingSystem.Reflection;usingBenchmarkDotNet.Attributes;publicclassBench{staticintx;[Benchmark]publicvoidFinally(){try{try{try{try{try{try{try{try{try{try{thrownewException();}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}catch{}}}
@EgorBotEgorBot
mentioned this pull request
May 4, 2025

Open

@jkotas
jkotas
commented
May 4, 2025

Copy link


Copy Markdown

MemberAuthor
@EgorBot -windows_intel -intel
usingSystem.Reflection;usingBenchmarkDotNet.Attributes;publicclassBench{staticintx;[Benchmark]publicvoidFinally(){try{try{try{try{try{try{try{try{try{try{thrownewException();}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}catch{}}}
@EgorBotEgorBot
mentioned this pull request
May 4, 2025

Open

@jkotas
jkotas
commented
May 4, 2025

Copy link


Copy Markdown

MemberAuthor
@MihuBot
@MihuBotMihuBot
mentioned this pull request
May 4, 2025

Open

@jkotas
jkotas
commented
May 5, 2025•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
Perf results summary:
  • 0.6% code size improvement on average
  • Artificial micro-benchmark to measure the cost of saving and restoring all registers around funclet invocation is very noisy. The results vary between 10% improvement and 5% regression.
@jkotas
jkotas
commented
May 5, 2025

Copy link


Copy Markdown

MemberAuthor
@janvorli @dotnet/jit-contrib PTLA. This PR is for x64 only. I plan to switch other platforms to the unified scheme in subsequent PRs.
janvorli
janvorli
reviewed
May 5, 2025
View reviewed changes
Comment threadsrc/coreclr/vm/amd64/AsmHelpers.asm
Comment threadsrc/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm
Outdated

@build-analysisbuild-analysisBot
mentioned this pull request
May 5, 2025

Closed

janvorli
janvorli
approved these changes
May 5, 2025
View reviewed changes
@janvorlijanvorli
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

LGTM, thank you!
BruceForstall
BruceForstall
approved these changes
May 5, 2025
View reviewed changes
Comment threadsrc/coreclr/jit/codegenxarch.cpp
@BruceForstall
BruceForstall
commented
May 5, 2025

Copy link


Copy Markdown

Contributor
I suggest running outerloop tests.
@jkotas
jkotas
commented
May 6, 2025

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-coreclr outerloop
@azure-pipelines
azure-pipelinesBot
commented
May 6, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@build-analysisbuild-analysisBot
mentioned this pull request
May 6, 2025

Open

3 tasks
@jkotas
jkotas
merged commit 33b5215
into
dotnet:mainMay 10, 2025
@jkotas
jkotas
deleted the

funclets

branch
May 10, 2025 05:41
@LoopedBard3LoopedBard3
mentioned this pull request
May 13, 2025

Closed

@jkotasjkotas
mentioned this pull request
May 19, 2025

Closed

@LoopedBard3LoopedBard3
mentioned this pull request
May 20, 2025

Closed

@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jun 9, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@EgorBoEgorBoEgorBo left review comments
Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@janvorlijanvorlijanvorli approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovskyMichalStrehovsky is a code owner

+1 more reviewer

@BruceForstallBruceForstallBruceForstall approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jkotasjkotas

Labels

area-ExceptionHandling-coreclronly use for closed issues

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

5 participants

@jkotas@BruceForstall@EgorBo@janvorli

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Smaller funclet prologs/epilogs (x64) - #115284
Merged
jkotas merged 6 commits into
dotnet:mainfrom 
jkotas:funclets
May 10, 2025
Merged
Smaller funclet prologs/epilogs (x64)#115284
jkotas merged 6 commits into
dotnet:mainfrom 
jkotas:funclets
Conversation
@jkotas
@jkotasjkotas
commented
May 4, 2025

Copy link


Copy Markdown

Member
  • Allow funclets to trash non-volatile registers to make funclet prologs/epilogs smaller
  • Restore non-volatile registers before non-filter funclet calls
  • Unify funclet ABI between NAOT and non-NAOT
@jkotas
Smaller funclets prologs/epilogs (x64)
3d10555
- Allow funclets to trash non-volatile registers to make funclet prologs/epilogs smaller
- Restore non-volatile registers before non-filter funclet calls
- Unify funclet ABI between NAOT and non-NAOT
CopilotAI
review requested due to automatic review settings
May 4, 2025 18:57
@ghostghost
added
the 
needs-area-label
An area label is needed to ensure this gets routed to the appropriate area owners
label
May 4, 2025
@jkotasjkotas
added

area-ExceptionHandling-coreclr
only use for closed issues
and removed

needs-area-label
An area label is needed to ensure this gets routed to the appropriate area owners
labels
May 4, 2025
CopilotAI
reviewed
May 4, 2025
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull Request Overview
This PR refactors the funclet prolog/epilog sequences for x64 to reduce their size while unifying the ABI between NAOT and non-NAOT builds.
  • Removed redundant saving/restoring of non-volatile registers in funclets
  • Updated stack frame allocation and alignment computations
  • Adjusted documentation to reflect the revised behavior and design decisions
Reviewed Changes
Copilot reviewed 2 out of 6 changed files in this pull request and generated 2 comments.
FileDescription
src/coreclr/jit/codegenxarch.cppRemoved EBP and callee-register save/restore logic; updated frame allocation calculation using REG_NA
docs/design/coreclr/botr/clr-abi.mdRevised description of register preservation behavior in funclets
Files not reviewed (4)
  • src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.S: Language not supported
  • src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm: Language not supported
  • src/coreclr/vm/amd64/AsmHelpers.asm: Language not supported
  • src/coreclr/vm/amd64/asmhelpers.S: Language not supported
Comment threadsrc/coreclr/jit/codegenxarch.cpp
Outdated

Comment threaddocs/design/coreclr/botr/clr-abi.md
@jkotasjkotas
changed the title
Smaller funclets prologs/epilogs (x64)Smaller funclet prologs/epilogs (x64)May 4, 2025

This was referenced May 4, 2025

Open


Open


Open

@jkotas
jkotas
commented
May 4, 2025

Copy link


Copy Markdown

MemberAuthor
@EgorBot -windows_intel -amd
usingSystem.Reflection;usingBenchmarkDotNet.Attributes;publicclassBench{staticintx;[Benchmark]publicvoidFinally(){try{try{try{try{try{try{try{try{try{try{thrownewException();}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}catch{}}}
@EgorBotEgorBot
mentioned this pull request
May 4, 2025

Open

@jkotas
jkotas
commented
May 4, 2025

Copy link


Copy Markdown

MemberAuthor
@EgorBot -windows_intel -intel
usingSystem.Reflection;usingBenchmarkDotNet.Attributes;publicclassBench{staticintx;[Benchmark]publicvoidFinally(){try{try{try{try{try{try{try{try{try{try{thrownewException();}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}catch{}}}
@EgorBotEgorBot
mentioned this pull request
May 4, 2025

Open

@jkotas
jkotas
commented
May 4, 2025

Copy link


Copy Markdown

MemberAuthor
@MihuBot
@MihuBotMihuBot
mentioned this pull request
May 4, 2025

Open

@jkotas
jkotas
commented
May 5, 2025•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
Perf results summary:
  • 0.6% code size improvement on average
  • Artificial micro-benchmark to measure the cost of saving and restoring all registers around funclet invocation is very noisy. The results vary between 10% improvement and 5% regression.
@jkotas
jkotas
commented
May 5, 2025

Copy link


Copy Markdown

MemberAuthor
@janvorli @dotnet/jit-contrib PTLA. This PR is for x64 only. I plan to switch other platforms to the unified scheme in subsequent PRs.
janvorli
janvorli
reviewed
May 5, 2025
View reviewed changes
Comment threadsrc/coreclr/vm/amd64/AsmHelpers.asm
Comment threadsrc/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm
Outdated

@build-analysisbuild-analysisBot
mentioned this pull request
May 5, 2025

Closed

janvorli
janvorli
approved these changes
May 5, 2025
View reviewed changes
@janvorlijanvorli
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

LGTM, thank you!
BruceForstall
BruceForstall
approved these changes
May 5, 2025
View reviewed changes
Comment threadsrc/coreclr/jit/codegenxarch.cpp
@BruceForstall
BruceForstall
commented
May 5, 2025

Copy link


Copy Markdown

Contributor
I suggest running outerloop tests.
@jkotas
jkotas
commented
May 6, 2025

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-coreclr outerloop
@azure-pipelines
azure-pipelinesBot
commented
May 6, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@build-analysisbuild-analysisBot
mentioned this pull request
May 6, 2025

Open

3 tasks
@jkotas
jkotas
merged commit 33b5215
into
dotnet:mainMay 10, 2025
@jkotas
jkotas
deleted the

funclets

branch
May 10, 2025 05:41
@LoopedBard3LoopedBard3
mentioned this pull request
May 13, 2025

Closed

@jkotasjkotas
mentioned this pull request
May 19, 2025

Closed

@LoopedBard3LoopedBard3
mentioned this pull request
May 20, 2025

Closed

@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jun 9, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@EgorBoEgorBoEgorBo left review comments
Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@janvorlijanvorlijanvorli approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovskyMichalStrehovsky is a code owner

+1 more reviewer

@BruceForstallBruceForstallBruceForstall approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jkotasjkotas

Labels

area-ExceptionHandling-coreclronly use for closed issues

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

5 participants

@jkotas@BruceForstall@EgorBo@janvorli

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Smaller funclet prologs/epilogs (x64) - #115284
Merged
jkotas merged 6 commits into
dotnet:mainfrom 
jkotas:funclets
May 10, 2025
Merged
Smaller funclet prologs/epilogs (x64)#115284
jkotas merged 6 commits into
dotnet:mainfrom 
jkotas:funclets
Conversation
@jkotas
@jkotasjkotas
commented
May 4, 2025

Copy link


Copy Markdown

Member
  • Allow funclets to trash non-volatile registers to make funclet prologs/epilogs smaller
  • Restore non-volatile registers before non-filter funclet calls
  • Unify funclet ABI between NAOT and non-NAOT
@jkotas
Smaller funclets prologs/epilogs (x64)
3d10555
- Allow funclets to trash non-volatile registers to make funclet prologs/epilogs smaller
- Restore non-volatile registers before non-filter funclet calls
- Unify funclet ABI between NAOT and non-NAOT
CopilotAI
review requested due to automatic review settings
May 4, 2025 18:57
@ghostghost
added
the 
needs-area-label
An area label is needed to ensure this gets routed to the appropriate area owners
label
May 4, 2025
@jkotasjkotas
added

area-ExceptionHandling-coreclr
only use for closed issues
and removed

needs-area-label
An area label is needed to ensure this gets routed to the appropriate area owners
labels
May 4, 2025
CopilotAI
reviewed
May 4, 2025
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull Request Overview
This PR refactors the funclet prolog/epilog sequences for x64 to reduce their size while unifying the ABI between NAOT and non-NAOT builds.
  • Removed redundant saving/restoring of non-volatile registers in funclets
  • Updated stack frame allocation and alignment computations
  • Adjusted documentation to reflect the revised behavior and design decisions
Reviewed Changes
Copilot reviewed 2 out of 6 changed files in this pull request and generated 2 comments.
FileDescription
src/coreclr/jit/codegenxarch.cppRemoved EBP and callee-register save/restore logic; updated frame allocation calculation using REG_NA
docs/design/coreclr/botr/clr-abi.mdRevised description of register preservation behavior in funclets
Files not reviewed (4)
  • src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.S: Language not supported
  • src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm: Language not supported
  • src/coreclr/vm/amd64/AsmHelpers.asm: Language not supported
  • src/coreclr/vm/amd64/asmhelpers.S: Language not supported
Comment threadsrc/coreclr/jit/codegenxarch.cpp
Outdated

Comment threaddocs/design/coreclr/botr/clr-abi.md
@jkotasjkotas
changed the title
Smaller funclets prologs/epilogs (x64)Smaller funclet prologs/epilogs (x64)May 4, 2025

This was referenced May 4, 2025

Open


Open


Open

@jkotas
jkotas
commented
May 4, 2025

Copy link


Copy Markdown

MemberAuthor
@EgorBot -windows_intel -amd
usingSystem.Reflection;usingBenchmarkDotNet.Attributes;publicclassBench{staticintx;[Benchmark]publicvoidFinally(){try{try{try{try{try{try{try{try{try{try{thrownewException();}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}catch{}}}
@EgorBotEgorBot
mentioned this pull request
May 4, 2025

Open

@jkotas
jkotas
commented
May 4, 2025

Copy link


Copy Markdown

MemberAuthor
@EgorBot -windows_intel -intel
usingSystem.Reflection;usingBenchmarkDotNet.Attributes;publicclassBench{staticintx;[Benchmark]publicvoidFinally(){try{try{try{try{try{try{try{try{try{try{thrownewException();}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}catch{}}}
@EgorBotEgorBot
mentioned this pull request
May 4, 2025

Open

@jkotas
jkotas
commented
May 4, 2025

Copy link


Copy Markdown

MemberAuthor
@MihuBot
@MihuBotMihuBot
mentioned this pull request
May 4, 2025

Open

@jkotas
jkotas
commented
May 5, 2025•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
Perf results summary:
  • 0.6% code size improvement on average
  • Artificial micro-benchmark to measure the cost of saving and restoring all registers around funclet invocation is very noisy. The results vary between 10% improvement and 5% regression.
@jkotas
jkotas
commented
May 5, 2025

Copy link


Copy Markdown

MemberAuthor
@janvorli @dotnet/jit-contrib PTLA. This PR is for x64 only. I plan to switch other platforms to the unified scheme in subsequent PRs.
janvorli
janvorli
reviewed
May 5, 2025
View reviewed changes
Comment threadsrc/coreclr/vm/amd64/AsmHelpers.asm
Comment threadsrc/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm
Outdated

@build-analysisbuild-analysisBot
mentioned this pull request
May 5, 2025

Closed

janvorli
janvorli
approved these changes
May 5, 2025
View reviewed changes
@janvorlijanvorli
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

LGTM, thank you!
BruceForstall
BruceForstall
approved these changes
May 5, 2025
View reviewed changes
Comment threadsrc/coreclr/jit/codegenxarch.cpp
@BruceForstall
BruceForstall
commented
May 5, 2025

Copy link


Copy Markdown

Contributor
I suggest running outerloop tests.
@jkotas
jkotas
commented
May 6, 2025

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-coreclr outerloop
@azure-pipelines
azure-pipelinesBot
commented
May 6, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@build-analysisbuild-analysisBot
mentioned this pull request
May 6, 2025

Open

3 tasks
@jkotas
jkotas
merged commit 33b5215
into
dotnet:mainMay 10, 2025
@jkotas
jkotas
deleted the

funclets

branch
May 10, 2025 05:41
@LoopedBard3LoopedBard3
mentioned this pull request
May 13, 2025

Closed

@jkotasjkotas
mentioned this pull request
May 19, 2025

Closed

@LoopedBard3LoopedBard3
mentioned this pull request
May 20, 2025

Closed

@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jun 9, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@EgorBoEgorBoEgorBo left review comments
Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@janvorlijanvorlijanvorli approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovskyMichalStrehovsky is a code owner

+1 more reviewer

@BruceForstallBruceForstallBruceForstall approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jkotasjkotas

Labels

area-ExceptionHandling-coreclronly use for closed issues

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

5 participants

@jkotas@BruceForstall@EgorBo@janvorli

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Smaller funclet prologs/epilogs (x64) - #115284
Merged
jkotas merged 6 commits into
dotnet:mainfrom 
jkotas:funclets
May 10, 2025
Merged
Smaller funclet prologs/epilogs (x64)#115284
jkotas merged 6 commits into
dotnet:mainfrom 
jkotas:funclets
Conversation
@jkotas
@jkotasjkotas
commented
May 4, 2025

Copy link


Copy Markdown

Member
  • Allow funclets to trash non-volatile registers to make funclet prologs/epilogs smaller
  • Restore non-volatile registers before non-filter funclet calls
  • Unify funclet ABI between NAOT and non-NAOT
@jkotas
Smaller funclets prologs/epilogs (x64)
3d10555
- Allow funclets to trash non-volatile registers to make funclet prologs/epilogs smaller
- Restore non-volatile registers before non-filter funclet calls
- Unify funclet ABI between NAOT and non-NAOT
CopilotAI
review requested due to automatic review settings
May 4, 2025 18:57
@ghostghost
added
the 
needs-area-label
An area label is needed to ensure this gets routed to the appropriate area owners
label
May 4, 2025
@jkotasjkotas
added

area-ExceptionHandling-coreclr
only use for closed issues
and removed

needs-area-label
An area label is needed to ensure this gets routed to the appropriate area owners
labels
May 4, 2025
CopilotAI
reviewed
May 4, 2025
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull Request Overview
This PR refactors the funclet prolog/epilog sequences for x64 to reduce their size while unifying the ABI between NAOT and non-NAOT builds.
  • Removed redundant saving/restoring of non-volatile registers in funclets
  • Updated stack frame allocation and alignment computations
  • Adjusted documentation to reflect the revised behavior and design decisions
Reviewed Changes
Copilot reviewed 2 out of 6 changed files in this pull request and generated 2 comments.
FileDescription
src/coreclr/jit/codegenxarch.cppRemoved EBP and callee-register save/restore logic; updated frame allocation calculation using REG_NA
docs/design/coreclr/botr/clr-abi.mdRevised description of register preservation behavior in funclets
Files not reviewed (4)
  • src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.S: Language not supported
  • src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm: Language not supported
  • src/coreclr/vm/amd64/AsmHelpers.asm: Language not supported
  • src/coreclr/vm/amd64/asmhelpers.S: Language not supported
Comment threadsrc/coreclr/jit/codegenxarch.cpp
Outdated

Comment threaddocs/design/coreclr/botr/clr-abi.md
@jkotasjkotas
changed the title
Smaller funclets prologs/epilogs (x64)Smaller funclet prologs/epilogs (x64)May 4, 2025

This was referenced May 4, 2025

Open


Open


Open

@jkotas
jkotas
commented
May 4, 2025

Copy link


Copy Markdown

MemberAuthor
@EgorBot -windows_intel -amd
usingSystem.Reflection;usingBenchmarkDotNet.Attributes;publicclassBench{staticintx;[Benchmark]publicvoidFinally(){try{try{try{try{try{try{try{try{try{try{thrownewException();}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}catch{}}}
@EgorBotEgorBot
mentioned this pull request
May 4, 2025

Open

@jkotas
jkotas
commented
May 4, 2025

Copy link


Copy Markdown

MemberAuthor
@EgorBot -windows_intel -intel
usingSystem.Reflection;usingBenchmarkDotNet.Attributes;publicclassBench{staticintx;[Benchmark]publicvoidFinally(){try{try{try{try{try{try{try{try{try{try{thrownewException();}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}catch{}}}
@EgorBotEgorBot
mentioned this pull request
May 4, 2025

Open

@jkotas
jkotas
commented
May 4, 2025

Copy link


Copy Markdown

MemberAuthor
@MihuBot
@MihuBotMihuBot
mentioned this pull request
May 4, 2025

Open

@jkotas
jkotas
commented
May 5, 2025•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
Perf results summary:
  • 0.6% code size improvement on average
  • Artificial micro-benchmark to measure the cost of saving and restoring all registers around funclet invocation is very noisy. The results vary between 10% improvement and 5% regression.
@jkotas
jkotas
commented
May 5, 2025

Copy link


Copy Markdown

MemberAuthor
@janvorli @dotnet/jit-contrib PTLA. This PR is for x64 only. I plan to switch other platforms to the unified scheme in subsequent PRs.
janvorli
janvorli
reviewed
May 5, 2025
View reviewed changes
Comment threadsrc/coreclr/vm/amd64/AsmHelpers.asm
Comment threadsrc/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm
Outdated

@build-analysisbuild-analysisBot
mentioned this pull request
May 5, 2025

Closed

janvorli
janvorli
approved these changes
May 5, 2025
View reviewed changes
@janvorlijanvorli
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

LGTM, thank you!
BruceForstall
BruceForstall
approved these changes
May 5, 2025
View reviewed changes
Comment threadsrc/coreclr/jit/codegenxarch.cpp
@BruceForstall
BruceForstall
commented
May 5, 2025

Copy link


Copy Markdown

Contributor
I suggest running outerloop tests.
@jkotas
jkotas
commented
May 6, 2025

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-coreclr outerloop
@azure-pipelines
azure-pipelinesBot
commented
May 6, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@build-analysisbuild-analysisBot
mentioned this pull request
May 6, 2025

Open

3 tasks
@jkotas
jkotas
merged commit 33b5215
into
dotnet:mainMay 10, 2025
@jkotas
jkotas
deleted the

funclets

branch
May 10, 2025 05:41
@LoopedBard3LoopedBard3
mentioned this pull request
May 13, 2025

Closed

@jkotasjkotas
mentioned this pull request
May 19, 2025

Closed

@LoopedBard3LoopedBard3
mentioned this pull request
May 20, 2025

Closed

@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jun 9, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@EgorBoEgorBoEgorBo left review comments
Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@janvorlijanvorlijanvorli approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovskyMichalStrehovsky is a code owner

+1 more reviewer

@BruceForstallBruceForstallBruceForstall approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jkotasjkotas

Labels

area-ExceptionHandling-coreclronly use for closed issues

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

5 participants

@jkotas@BruceForstall@EgorBo@janvorli

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Smaller funclet prologs/epilogs (x64) - #115284
Merged
jkotas merged 6 commits into
dotnet:mainfrom 
jkotas:funclets
May 10, 2025
Merged
Smaller funclet prologs/epilogs (x64)#115284
jkotas merged 6 commits into
dotnet:mainfrom 
jkotas:funclets
Conversation
@jkotas
@jkotasjkotas
commented
May 4, 2025

Copy link


Copy Markdown

Member
  • Allow funclets to trash non-volatile registers to make funclet prologs/epilogs smaller
  • Restore non-volatile registers before non-filter funclet calls
  • Unify funclet ABI between NAOT and non-NAOT
@jkotas
Smaller funclets prologs/epilogs (x64)
3d10555
- Allow funclets to trash non-volatile registers to make funclet prologs/epilogs smaller
- Restore non-volatile registers before non-filter funclet calls
- Unify funclet ABI between NAOT and non-NAOT
CopilotAI
review requested due to automatic review settings
May 4, 2025 18:57
@ghostghost
added
the 
needs-area-label
An area label is needed to ensure this gets routed to the appropriate area owners
label
May 4, 2025
@jkotasjkotas
added

area-ExceptionHandling-coreclr
only use for closed issues
and removed

needs-area-label
An area label is needed to ensure this gets routed to the appropriate area owners
labels
May 4, 2025
CopilotAI
reviewed
May 4, 2025
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull Request Overview
This PR refactors the funclet prolog/epilog sequences for x64 to reduce their size while unifying the ABI between NAOT and non-NAOT builds.
  • Removed redundant saving/restoring of non-volatile registers in funclets
  • Updated stack frame allocation and alignment computations
  • Adjusted documentation to reflect the revised behavior and design decisions
Reviewed Changes
Copilot reviewed 2 out of 6 changed files in this pull request and generated 2 comments.
FileDescription
src/coreclr/jit/codegenxarch.cppRemoved EBP and callee-register save/restore logic; updated frame allocation calculation using REG_NA
docs/design/coreclr/botr/clr-abi.mdRevised description of register preservation behavior in funclets
Files not reviewed (4)
  • src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.S: Language not supported
  • src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm: Language not supported
  • src/coreclr/vm/amd64/AsmHelpers.asm: Language not supported
  • src/coreclr/vm/amd64/asmhelpers.S: Language not supported
Comment threadsrc/coreclr/jit/codegenxarch.cpp
Outdated

Comment threaddocs/design/coreclr/botr/clr-abi.md
@jkotasjkotas
changed the title
Smaller funclets prologs/epilogs (x64)Smaller funclet prologs/epilogs (x64)May 4, 2025

This was referenced May 4, 2025

Open


Open


Open

@jkotas
jkotas
commented
May 4, 2025

Copy link


Copy Markdown

MemberAuthor
@EgorBot -windows_intel -amd
usingSystem.Reflection;usingBenchmarkDotNet.Attributes;publicclassBench{staticintx;[Benchmark]publicvoidFinally(){try{try{try{try{try{try{try{try{try{try{thrownewException();}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}catch{}}}
@EgorBotEgorBot
mentioned this pull request
May 4, 2025

Open

@jkotas
jkotas
commented
May 4, 2025

Copy link


Copy Markdown

MemberAuthor
@EgorBot -windows_intel -intel
usingSystem.Reflection;usingBenchmarkDotNet.Attributes;publicclassBench{staticintx;[Benchmark]publicvoidFinally(){try{try{try{try{try{try{try{try{try{try{thrownewException();}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}catch{}}}
@EgorBotEgorBot
mentioned this pull request
May 4, 2025

Open

@jkotas
jkotas
commented
May 4, 2025

Copy link


Copy Markdown

MemberAuthor
@MihuBot
@MihuBotMihuBot
mentioned this pull request
May 4, 2025

Open

@jkotas
jkotas
commented
May 5, 2025•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
Perf results summary:
  • 0.6% code size improvement on average
  • Artificial micro-benchmark to measure the cost of saving and restoring all registers around funclet invocation is very noisy. The results vary between 10% improvement and 5% regression.
@jkotas
jkotas
commented
May 5, 2025

Copy link


Copy Markdown

MemberAuthor
@janvorli @dotnet/jit-contrib PTLA. This PR is for x64 only. I plan to switch other platforms to the unified scheme in subsequent PRs.
janvorli
janvorli
reviewed
May 5, 2025
View reviewed changes
Comment threadsrc/coreclr/vm/amd64/AsmHelpers.asm
Comment threadsrc/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm
Outdated

@build-analysisbuild-analysisBot
mentioned this pull request
May 5, 2025

Closed

janvorli
janvorli
approved these changes
May 5, 2025
View reviewed changes
@janvorlijanvorli
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

LGTM, thank you!
BruceForstall
BruceForstall
approved these changes
May 5, 2025
View reviewed changes
Comment threadsrc/coreclr/jit/codegenxarch.cpp
@BruceForstall
BruceForstall
commented
May 5, 2025

Copy link


Copy Markdown

Contributor
I suggest running outerloop tests.
@jkotas
jkotas
commented
May 6, 2025

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-coreclr outerloop
@azure-pipelines
azure-pipelinesBot
commented
May 6, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@build-analysisbuild-analysisBot
mentioned this pull request
May 6, 2025

Open

3 tasks
@jkotas
jkotas
merged commit 33b5215
into
dotnet:mainMay 10, 2025
@jkotas
jkotas
deleted the

funclets

branch
May 10, 2025 05:41
@LoopedBard3LoopedBard3
mentioned this pull request
May 13, 2025

Closed

@jkotasjkotas
mentioned this pull request
May 19, 2025

Closed

@LoopedBard3LoopedBard3
mentioned this pull request
May 20, 2025

Closed

@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jun 9, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@EgorBoEgorBoEgorBo left review comments
Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@janvorlijanvorlijanvorli approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovskyMichalStrehovsky is a code owner

+1 more reviewer

@BruceForstallBruceForstallBruceForstall approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jkotasjkotas

Labels

area-ExceptionHandling-coreclronly use for closed issues

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

5 participants

@jkotas@BruceForstall@EgorBo@janvorli

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Smaller funclet prologs/epilogs (x64) - #115284
Merged
jkotas merged 6 commits into
dotnet:mainfrom 
jkotas:funclets
May 10, 2025
Merged
Smaller funclet prologs/epilogs (x64)#115284
jkotas merged 6 commits into
dotnet:mainfrom 
jkotas:funclets
Conversation
@jkotas
@jkotasjkotas
commented
May 4, 2025

Copy link


Copy Markdown

Member
  • Allow funclets to trash non-volatile registers to make funclet prologs/epilogs smaller
  • Restore non-volatile registers before non-filter funclet calls
  • Unify funclet ABI between NAOT and non-NAOT
@jkotas
Smaller funclets prologs/epilogs (x64)
3d10555
- Allow funclets to trash non-volatile registers to make funclet prologs/epilogs smaller
- Restore non-volatile registers before non-filter funclet calls
- Unify funclet ABI between NAOT and non-NAOT
CopilotAI
review requested due to automatic review settings
May 4, 2025 18:57
@ghostghost
added
the 
needs-area-label
An area label is needed to ensure this gets routed to the appropriate area owners
label
May 4, 2025
@jkotasjkotas
added

area-ExceptionHandling-coreclr
only use for closed issues
and removed

needs-area-label
An area label is needed to ensure this gets routed to the appropriate area owners
labels
May 4, 2025
CopilotAI
reviewed
May 4, 2025
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull Request Overview
This PR refactors the funclet prolog/epilog sequences for x64 to reduce their size while unifying the ABI between NAOT and non-NAOT builds.
  • Removed redundant saving/restoring of non-volatile registers in funclets
  • Updated stack frame allocation and alignment computations
  • Adjusted documentation to reflect the revised behavior and design decisions
Reviewed Changes
Copilot reviewed 2 out of 6 changed files in this pull request and generated 2 comments.
FileDescription
src/coreclr/jit/codegenxarch.cppRemoved EBP and callee-register save/restore logic; updated frame allocation calculation using REG_NA
docs/design/coreclr/botr/clr-abi.mdRevised description of register preservation behavior in funclets
Files not reviewed (4)
  • src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.S: Language not supported
  • src/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm: Language not supported
  • src/coreclr/vm/amd64/AsmHelpers.asm: Language not supported
  • src/coreclr/vm/amd64/asmhelpers.S: Language not supported
Comment threadsrc/coreclr/jit/codegenxarch.cpp
Outdated

Comment threaddocs/design/coreclr/botr/clr-abi.md
@jkotasjkotas
changed the title
Smaller funclets prologs/epilogs (x64)Smaller funclet prologs/epilogs (x64)May 4, 2025

This was referenced May 4, 2025

Open


Open


Open

@jkotas
jkotas
commented
May 4, 2025

Copy link


Copy Markdown

MemberAuthor
@EgorBot -windows_intel -amd
usingSystem.Reflection;usingBenchmarkDotNet.Attributes;publicclassBench{staticintx;[Benchmark]publicvoidFinally(){try{try{try{try{try{try{try{try{try{try{thrownewException();}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}catch{}}}
@EgorBotEgorBot
mentioned this pull request
May 4, 2025

Open

@jkotas
jkotas
commented
May 4, 2025

Copy link


Copy Markdown

MemberAuthor
@EgorBot -windows_intel -intel
usingSystem.Reflection;usingBenchmarkDotNet.Attributes;publicclassBench{staticintx;[Benchmark]publicvoidFinally(){try{try{try{try{try{try{try{try{try{try{thrownewException();}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}finally{x++;}}catch{}}}
@EgorBotEgorBot
mentioned this pull request
May 4, 2025

Open

@jkotas
jkotas
commented
May 4, 2025

Copy link


Copy Markdown

MemberAuthor
@MihuBot
@MihuBotMihuBot
mentioned this pull request
May 4, 2025

Open

@jkotas
jkotas
commented
May 5, 2025•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
Perf results summary:
  • 0.6% code size improvement on average
  • Artificial micro-benchmark to measure the cost of saving and restoring all registers around funclet invocation is very noisy. The results vary between 10% improvement and 5% regression.
@jkotas
jkotas
commented
May 5, 2025

Copy link


Copy Markdown

MemberAuthor
@janvorli @dotnet/jit-contrib PTLA. This PR is for x64 only. I plan to switch other platforms to the unified scheme in subsequent PRs.
janvorli
janvorli
reviewed
May 5, 2025
View reviewed changes
Comment threadsrc/coreclr/vm/amd64/AsmHelpers.asm
Comment threadsrc/coreclr/nativeaot/Runtime/amd64/ExceptionHandling.asm
Outdated

@build-analysisbuild-analysisBot
mentioned this pull request
May 5, 2025

Closed

janvorli
janvorli
approved these changes
May 5, 2025
View reviewed changes
@janvorlijanvorli
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

LGTM, thank you!
BruceForstall
BruceForstall
approved these changes
May 5, 2025
View reviewed changes
Comment threadsrc/coreclr/jit/codegenxarch.cpp
@BruceForstall
BruceForstall
commented
May 5, 2025

Copy link


Copy Markdown

Contributor
I suggest running outerloop tests.
@jkotas
jkotas
commented
May 6, 2025

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-coreclr outerloop
@azure-pipelines
azure-pipelinesBot
commented
May 6, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@build-analysisbuild-analysisBot
mentioned this pull request
May 6, 2025

Open

3 tasks
@jkotas
jkotas
merged commit 33b5215
into
dotnet:mainMay 10, 2025
@jkotas
jkotas
deleted the

funclets

branch
May 10, 2025 05:41
@LoopedBard3LoopedBard3
mentioned this pull request
May 13, 2025

Closed

@jkotasjkotas
mentioned this pull request
May 19, 2025

Closed

@LoopedBard3LoopedBard3
mentioned this pull request
May 20, 2025

Closed

@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jun 9, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@EgorBoEgorBoEgorBo left review comments
Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@janvorlijanvorlijanvorli approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovskyMichalStrehovsky is a code owner

+1 more reviewer

@BruceForstallBruceForstallBruceForstall approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jkotasjkotas

Labels

area-ExceptionHandling-coreclronly use for closed issues

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

5 participants

@jkotas@BruceForstall@EgorBo@janvorli