[clr-interp] Use struct parameter for InvokeUnmanagedCalliWithTransition - #127543

Closed
clamp03 wants to merge 4 commits into
dotnet:mainfrom
clamp03:bundle_args
Closed

[clr-interp] Use struct parameter for InvokeUnmanagedCalliWithTransition#127543
clamp03 wants to merge 4 commits into
dotnet:mainfrom
clamp03:bundle_args

Conversation

@clamp03

Copy link
Copy Markdown
Member

On ARM32, bundling arguments in a struct forces register passing, ensuring the SP value saved by SAVE_THE_LOWEST_SP matches what stack walking reports.

On ARM32, bundling arguments in a struct forces register passing, ensuring
the SP value saved by SAVE_THE_LOWEST_SP matches what stack walking reports.
@clamp03clamp03 self-assigned this Apr 29, 2026
@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Apr 29, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @BrzVlad, @janvorli, @kg
See info in area-owners.md if you want to be subscribed.

@am11

am11 commented Apr 29, 2026

Copy link
Copy Markdown
Member

@clamp03, if you get a chance, could you also run interpreter tests on riscv64? If something fails, please file an issue with build and test commands. I will take from there (I was struggling to figure out commands with --bootstrap earlier but code was ported to riscv64 to the extent that 'it just compiles' 😅)

@clamp03

Copy link
Copy Markdown
MemberAuthor

@clamp03, if you get a chance, could you also run interpreter tests on riscv64? If something fails, please file an issue with build and test commands. I will take from there (I was struggling to figure out commands with --bootstrap earlier but code was ported to riscv64 to the extent that 'it just compiles' 😅)

@am11 Okay, I'll check the interpreter tests on riscv64 after May 5th, as we have a holiday until then. Since I don't have an available riscv64 device now, I'll run the tests using QEMU. Thanks!

@clamp03

Copy link
Copy Markdown
MemberAuthor

@BrzVlad@davidwrighton@janvorli Could you please review this PR? Thank you.

@BrzVlad

Copy link
Copy Markdown
Member

This seems fragile. Wouldn't it be better to save the FP instead and identify the native frame by this, given this wouldn't change and we wouldn't have to bother with updating every time sp changes ? @janvorli ?

@janvorli

Copy link
Copy Markdown
Member

This seems fragile. Wouldn't it be better to save the FP instead and identify the native frame by this, given this wouldn't change and we wouldn't have to bother with updating every time sp changes ? @janvorli ?

@BrzVlad we did that already for other cases, so this just adds another one. I agree it is ugly. Looking again at where we use the saved SP in the current state, there is one place where we pass it into a call that ends up ignoring it (which wasn't the case when we have decided to use this way of mitigating the SP issue) and then the only remaining one is the ResumeAfterCatch. It seems that one can be modified to use FP as you suggested since we compile with -fno-omit-frame-pointer.

So it seems we can move to using FP instead and get rid of this passing via structs.

@janvorli

Copy link
Copy Markdown
Member

@clamp03 let me try the change that @BrzVlad suggested. If that works, we can close this PR as it would not be needed.

@jkotas

Copy link
Copy Markdown
Member

we compile with -fno-omit-frame-pointer.

That's fragile in a different way. -fno-omit-frame-pointer is not a thing on Windows x64, and the optimizations can setup the FP in non-obvious ways.

@janvorli

Copy link
Copy Markdown
Member

@jkotas on Windows, you are right. We can keep using the SP. On Unix, it seems that it is required for the compiler to not to reuse FP for anything else when -fno-omit-frame-pointer is specified and the function in question is not inlined in some other that is compiled without the -fno-omit-frame-pointer. Both LLVM and GCC marks the frame register register as reserved in such case. Copilot has provided me with links to the source locations in LLVM/GCC and also to related abi docs that seem to confirm that.

However, yesterday I have resumed work on changing the ResumeAfterCatch to get rid of the explicit native frames unwinding and using C++ exception like we do on WASM. That would remove the need for storing the SP or FP completely.

@jakobbotsch

Copy link
Copy Markdown
Member

What is the status here? Did you land the change mentioned above @janvorli?

@janvorli

Copy link
Copy Markdown
Member

@jakobbotsch the change is in PR #128728, I am still fixing some issues with it.

@clamp03

Copy link
Copy Markdown
MemberAuthor

I will close this PR once PR #128728 is merged.

@JulieLeeMSFT
JulieLeeMSFT marked this pull request as draft June 2, 2026 18:11
@janvorli

Copy link
Copy Markdown
Member

@clamp03 I've just merged my PR in.

@clamp03

clamp03 commented Jun 9, 2026

Copy link
Copy Markdown
MemberAuthor

@clamp03 I've just merged my PR in.

Thank you so much!

@janvorli + I will check interpreter on arm32 and then revert all struct parameters in interpexec.

@clamp03clamp03 closed this Jun 9, 2026
@janvorli

Copy link
Copy Markdown
Member

Thank you @clamp03!

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

Labels

arch-arm32area-CodeGen-Interpreter-coreclrcommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@clamp03@am11@BrzVlad@janvorli@jkotas@jakobbotsch
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
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;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} 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

[clr-interp] Use struct parameter for InvokeUnmanagedCalliWithTransition - #127543

Closed
clamp03 wants to merge 4 commits into
dotnet:mainfrom
clamp03:bundle_args
Closed

[clr-interp] Use struct parameter for InvokeUnmanagedCalliWithTransition#127543
clamp03 wants to merge 4 commits into
dotnet:mainfrom
clamp03:bundle_args

Conversation

@clamp03

Copy link
Copy Markdown
Member

On ARM32, bundling arguments in a struct forces register passing, ensuring the SP value saved by SAVE_THE_LOWEST_SP matches what stack walking reports.

On ARM32, bundling arguments in a struct forces register passing, ensuring
the SP value saved by SAVE_THE_LOWEST_SP matches what stack walking reports.
@clamp03clamp03 self-assigned this Apr 29, 2026
@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Apr 29, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @BrzVlad, @janvorli, @kg
See info in area-owners.md if you want to be subscribed.

@am11

am11 commented Apr 29, 2026

Copy link
Copy Markdown
Member

@clamp03, if you get a chance, could you also run interpreter tests on riscv64? If something fails, please file an issue with build and test commands. I will take from there (I was struggling to figure out commands with --bootstrap earlier but code was ported to riscv64 to the extent that 'it just compiles' 😅)

@clamp03

Copy link
Copy Markdown
MemberAuthor

@clamp03, if you get a chance, could you also run interpreter tests on riscv64? If something fails, please file an issue with build and test commands. I will take from there (I was struggling to figure out commands with --bootstrap earlier but code was ported to riscv64 to the extent that 'it just compiles' 😅)

@am11 Okay, I'll check the interpreter tests on riscv64 after May 5th, as we have a holiday until then. Since I don't have an available riscv64 device now, I'll run the tests using QEMU. Thanks!

@clamp03

Copy link
Copy Markdown
MemberAuthor

@BrzVlad@davidwrighton@janvorli Could you please review this PR? Thank you.

@BrzVlad

Copy link
Copy Markdown
Member

This seems fragile. Wouldn't it be better to save the FP instead and identify the native frame by this, given this wouldn't change and we wouldn't have to bother with updating every time sp changes ? @janvorli ?

@janvorli

Copy link
Copy Markdown
Member

This seems fragile. Wouldn't it be better to save the FP instead and identify the native frame by this, given this wouldn't change and we wouldn't have to bother with updating every time sp changes ? @janvorli ?

@BrzVlad we did that already for other cases, so this just adds another one. I agree it is ugly. Looking again at where we use the saved SP in the current state, there is one place where we pass it into a call that ends up ignoring it (which wasn't the case when we have decided to use this way of mitigating the SP issue) and then the only remaining one is the ResumeAfterCatch. It seems that one can be modified to use FP as you suggested since we compile with -fno-omit-frame-pointer.

So it seems we can move to using FP instead and get rid of this passing via structs.

@janvorli

Copy link
Copy Markdown
Member

@clamp03 let me try the change that @BrzVlad suggested. If that works, we can close this PR as it would not be needed.

@jkotas

Copy link
Copy Markdown
Member

we compile with -fno-omit-frame-pointer.

That's fragile in a different way. -fno-omit-frame-pointer is not a thing on Windows x64, and the optimizations can setup the FP in non-obvious ways.

@janvorli

Copy link
Copy Markdown
Member

@jkotas on Windows, you are right. We can keep using the SP. On Unix, it seems that it is required for the compiler to not to reuse FP for anything else when -fno-omit-frame-pointer is specified and the function in question is not inlined in some other that is compiled without the -fno-omit-frame-pointer. Both LLVM and GCC marks the frame register register as reserved in such case. Copilot has provided me with links to the source locations in LLVM/GCC and also to related abi docs that seem to confirm that.

However, yesterday I have resumed work on changing the ResumeAfterCatch to get rid of the explicit native frames unwinding and using C++ exception like we do on WASM. That would remove the need for storing the SP or FP completely.

@jakobbotsch

Copy link
Copy Markdown
Member

What is the status here? Did you land the change mentioned above @janvorli?

@janvorli

Copy link
Copy Markdown
Member

@jakobbotsch the change is in PR #128728, I am still fixing some issues with it.

@clamp03

Copy link
Copy Markdown
MemberAuthor

I will close this PR once PR #128728 is merged.

@JulieLeeMSFT
JulieLeeMSFT marked this pull request as draft June 2, 2026 18:11
@janvorli

Copy link
Copy Markdown
Member

@clamp03 I've just merged my PR in.

@clamp03

clamp03 commented Jun 9, 2026

Copy link
Copy Markdown
MemberAuthor

@clamp03 I've just merged my PR in.

Thank you so much!

@janvorli + I will check interpreter on arm32 and then revert all struct parameters in interpexec.

@clamp03clamp03 closed this Jun 9, 2026
@janvorli

Copy link
Copy Markdown
Member

Thank you @clamp03!

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

Labels

arch-arm32area-CodeGen-Interpreter-coreclrcommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

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

[clr-interp] Use struct parameter for InvokeUnmanagedCalliWithTransition - #127543

Closed
clamp03 wants to merge 4 commits into
dotnet:mainfrom
clamp03:bundle_args
Closed

[clr-interp] Use struct parameter for InvokeUnmanagedCalliWithTransition#127543
clamp03 wants to merge 4 commits into
dotnet:mainfrom
clamp03:bundle_args

Conversation

@clamp03

Copy link
Copy Markdown
Member

On ARM32, bundling arguments in a struct forces register passing, ensuring the SP value saved by SAVE_THE_LOWEST_SP matches what stack walking reports.

On ARM32, bundling arguments in a struct forces register passing, ensuring
the SP value saved by SAVE_THE_LOWEST_SP matches what stack walking reports.
@clamp03clamp03 self-assigned this Apr 29, 2026
@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Apr 29, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @BrzVlad, @janvorli, @kg
See info in area-owners.md if you want to be subscribed.

@am11

am11 commented Apr 29, 2026

Copy link
Copy Markdown
Member

@clamp03, if you get a chance, could you also run interpreter tests on riscv64? If something fails, please file an issue with build and test commands. I will take from there (I was struggling to figure out commands with --bootstrap earlier but code was ported to riscv64 to the extent that 'it just compiles' 😅)

@clamp03

Copy link
Copy Markdown
MemberAuthor

@clamp03, if you get a chance, could you also run interpreter tests on riscv64? If something fails, please file an issue with build and test commands. I will take from there (I was struggling to figure out commands with --bootstrap earlier but code was ported to riscv64 to the extent that 'it just compiles' 😅)

@am11 Okay, I'll check the interpreter tests on riscv64 after May 5th, as we have a holiday until then. Since I don't have an available riscv64 device now, I'll run the tests using QEMU. Thanks!

@clamp03

Copy link
Copy Markdown
MemberAuthor

@BrzVlad@davidwrighton@janvorli Could you please review this PR? Thank you.

@BrzVlad

Copy link
Copy Markdown
Member

This seems fragile. Wouldn't it be better to save the FP instead and identify the native frame by this, given this wouldn't change and we wouldn't have to bother with updating every time sp changes ? @janvorli ?

@janvorli

Copy link
Copy Markdown
Member

This seems fragile. Wouldn't it be better to save the FP instead and identify the native frame by this, given this wouldn't change and we wouldn't have to bother with updating every time sp changes ? @janvorli ?

@BrzVlad we did that already for other cases, so this just adds another one. I agree it is ugly. Looking again at where we use the saved SP in the current state, there is one place where we pass it into a call that ends up ignoring it (which wasn't the case when we have decided to use this way of mitigating the SP issue) and then the only remaining one is the ResumeAfterCatch. It seems that one can be modified to use FP as you suggested since we compile with -fno-omit-frame-pointer.

So it seems we can move to using FP instead and get rid of this passing via structs.

@janvorli

Copy link
Copy Markdown
Member

@clamp03 let me try the change that @BrzVlad suggested. If that works, we can close this PR as it would not be needed.

@jkotas

Copy link
Copy Markdown
Member

we compile with -fno-omit-frame-pointer.

That's fragile in a different way. -fno-omit-frame-pointer is not a thing on Windows x64, and the optimizations can setup the FP in non-obvious ways.

@janvorli

Copy link
Copy Markdown
Member

@jkotas on Windows, you are right. We can keep using the SP. On Unix, it seems that it is required for the compiler to not to reuse FP for anything else when -fno-omit-frame-pointer is specified and the function in question is not inlined in some other that is compiled without the -fno-omit-frame-pointer. Both LLVM and GCC marks the frame register register as reserved in such case. Copilot has provided me with links to the source locations in LLVM/GCC and also to related abi docs that seem to confirm that.

However, yesterday I have resumed work on changing the ResumeAfterCatch to get rid of the explicit native frames unwinding and using C++ exception like we do on WASM. That would remove the need for storing the SP or FP completely.

@jakobbotsch

Copy link
Copy Markdown
Member

What is the status here? Did you land the change mentioned above @janvorli?

@janvorli

Copy link
Copy Markdown
Member

@jakobbotsch the change is in PR #128728, I am still fixing some issues with it.

@clamp03

Copy link
Copy Markdown
MemberAuthor

I will close this PR once PR #128728 is merged.

@JulieLeeMSFT
JulieLeeMSFT marked this pull request as draft June 2, 2026 18:11
@janvorli

Copy link
Copy Markdown
Member

@clamp03 I've just merged my PR in.

@clamp03

clamp03 commented Jun 9, 2026

Copy link
Copy Markdown
MemberAuthor

@clamp03 I've just merged my PR in.

Thank you so much!

@janvorli + I will check interpreter on arm32 and then revert all struct parameters in interpexec.

@clamp03clamp03 closed this Jun 9, 2026
@janvorli

Copy link
Copy Markdown
Member

Thank you @clamp03!

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

Labels

arch-arm32area-CodeGen-Interpreter-coreclrcommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

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

[clr-interp] Use struct parameter for InvokeUnmanagedCalliWithTransition - #127543

Closed
clamp03 wants to merge 4 commits into
dotnet:mainfrom
clamp03:bundle_args
Closed

[clr-interp] Use struct parameter for InvokeUnmanagedCalliWithTransition#127543
clamp03 wants to merge 4 commits into
dotnet:mainfrom
clamp03:bundle_args

Conversation

@clamp03

Copy link
Copy Markdown
Member

On ARM32, bundling arguments in a struct forces register passing, ensuring the SP value saved by SAVE_THE_LOWEST_SP matches what stack walking reports.

On ARM32, bundling arguments in a struct forces register passing, ensuring
the SP value saved by SAVE_THE_LOWEST_SP matches what stack walking reports.
@clamp03clamp03 self-assigned this Apr 29, 2026
@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Apr 29, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @BrzVlad, @janvorli, @kg
See info in area-owners.md if you want to be subscribed.

@am11

am11 commented Apr 29, 2026

Copy link
Copy Markdown
Member

@clamp03, if you get a chance, could you also run interpreter tests on riscv64? If something fails, please file an issue with build and test commands. I will take from there (I was struggling to figure out commands with --bootstrap earlier but code was ported to riscv64 to the extent that 'it just compiles' 😅)

@clamp03

Copy link
Copy Markdown
MemberAuthor

@clamp03, if you get a chance, could you also run interpreter tests on riscv64? If something fails, please file an issue with build and test commands. I will take from there (I was struggling to figure out commands with --bootstrap earlier but code was ported to riscv64 to the extent that 'it just compiles' 😅)

@am11 Okay, I'll check the interpreter tests on riscv64 after May 5th, as we have a holiday until then. Since I don't have an available riscv64 device now, I'll run the tests using QEMU. Thanks!

@clamp03

Copy link
Copy Markdown
MemberAuthor

@BrzVlad@davidwrighton@janvorli Could you please review this PR? Thank you.

@BrzVlad

Copy link
Copy Markdown
Member

This seems fragile. Wouldn't it be better to save the FP instead and identify the native frame by this, given this wouldn't change and we wouldn't have to bother with updating every time sp changes ? @janvorli ?

@janvorli

Copy link
Copy Markdown
Member

This seems fragile. Wouldn't it be better to save the FP instead and identify the native frame by this, given this wouldn't change and we wouldn't have to bother with updating every time sp changes ? @janvorli ?

@BrzVlad we did that already for other cases, so this just adds another one. I agree it is ugly. Looking again at where we use the saved SP in the current state, there is one place where we pass it into a call that ends up ignoring it (which wasn't the case when we have decided to use this way of mitigating the SP issue) and then the only remaining one is the ResumeAfterCatch. It seems that one can be modified to use FP as you suggested since we compile with -fno-omit-frame-pointer.

So it seems we can move to using FP instead and get rid of this passing via structs.

@janvorli

Copy link
Copy Markdown
Member

@clamp03 let me try the change that @BrzVlad suggested. If that works, we can close this PR as it would not be needed.

@jkotas

Copy link
Copy Markdown
Member

we compile with -fno-omit-frame-pointer.

That's fragile in a different way. -fno-omit-frame-pointer is not a thing on Windows x64, and the optimizations can setup the FP in non-obvious ways.

@janvorli

Copy link
Copy Markdown
Member

@jkotas on Windows, you are right. We can keep using the SP. On Unix, it seems that it is required for the compiler to not to reuse FP for anything else when -fno-omit-frame-pointer is specified and the function in question is not inlined in some other that is compiled without the -fno-omit-frame-pointer. Both LLVM and GCC marks the frame register register as reserved in such case. Copilot has provided me with links to the source locations in LLVM/GCC and also to related abi docs that seem to confirm that.

However, yesterday I have resumed work on changing the ResumeAfterCatch to get rid of the explicit native frames unwinding and using C++ exception like we do on WASM. That would remove the need for storing the SP or FP completely.

@jakobbotsch

Copy link
Copy Markdown
Member

What is the status here? Did you land the change mentioned above @janvorli?

@janvorli

Copy link
Copy Markdown
Member

@jakobbotsch the change is in PR #128728, I am still fixing some issues with it.

@clamp03

Copy link
Copy Markdown
MemberAuthor

I will close this PR once PR #128728 is merged.

@JulieLeeMSFT
JulieLeeMSFT marked this pull request as draft June 2, 2026 18:11
@janvorli

Copy link
Copy Markdown
Member

@clamp03 I've just merged my PR in.

@clamp03

clamp03 commented Jun 9, 2026

Copy link
Copy Markdown
MemberAuthor

@clamp03 I've just merged my PR in.

Thank you so much!

@janvorli + I will check interpreter on arm32 and then revert all struct parameters in interpexec.

@clamp03clamp03 closed this Jun 9, 2026
@janvorli

Copy link
Copy Markdown
Member

Thank you @clamp03!

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

Labels

arch-arm32area-CodeGen-Interpreter-coreclrcommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@clamp03@am11@BrzVlad@janvorli@jkotas@jakobbotsch
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } 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

[clr-interp] Use struct parameter for InvokeUnmanagedCalliWithTransition - #127543

Closed
clamp03 wants to merge 4 commits into
dotnet:mainfrom
clamp03:bundle_args
Closed

[clr-interp] Use struct parameter for InvokeUnmanagedCalliWithTransition#127543
clamp03 wants to merge 4 commits into
dotnet:mainfrom
clamp03:bundle_args

Conversation

@clamp03

Copy link
Copy Markdown
Member

On ARM32, bundling arguments in a struct forces register passing, ensuring the SP value saved by SAVE_THE_LOWEST_SP matches what stack walking reports.

On ARM32, bundling arguments in a struct forces register passing, ensuring
the SP value saved by SAVE_THE_LOWEST_SP matches what stack walking reports.
@clamp03clamp03 self-assigned this Apr 29, 2026
@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Apr 29, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @BrzVlad, @janvorli, @kg
See info in area-owners.md if you want to be subscribed.

@am11

am11 commented Apr 29, 2026

Copy link
Copy Markdown
Member

@clamp03, if you get a chance, could you also run interpreter tests on riscv64? If something fails, please file an issue with build and test commands. I will take from there (I was struggling to figure out commands with --bootstrap earlier but code was ported to riscv64 to the extent that 'it just compiles' 😅)

@clamp03

Copy link
Copy Markdown
MemberAuthor

@clamp03, if you get a chance, could you also run interpreter tests on riscv64? If something fails, please file an issue with build and test commands. I will take from there (I was struggling to figure out commands with --bootstrap earlier but code was ported to riscv64 to the extent that 'it just compiles' 😅)

@am11 Okay, I'll check the interpreter tests on riscv64 after May 5th, as we have a holiday until then. Since I don't have an available riscv64 device now, I'll run the tests using QEMU. Thanks!

@clamp03

Copy link
Copy Markdown
MemberAuthor

@BrzVlad@davidwrighton@janvorli Could you please review this PR? Thank you.

@BrzVlad

Copy link
Copy Markdown
Member

This seems fragile. Wouldn't it be better to save the FP instead and identify the native frame by this, given this wouldn't change and we wouldn't have to bother with updating every time sp changes ? @janvorli ?

@janvorli

Copy link
Copy Markdown
Member

This seems fragile. Wouldn't it be better to save the FP instead and identify the native frame by this, given this wouldn't change and we wouldn't have to bother with updating every time sp changes ? @janvorli ?

@BrzVlad we did that already for other cases, so this just adds another one. I agree it is ugly. Looking again at where we use the saved SP in the current state, there is one place where we pass it into a call that ends up ignoring it (which wasn't the case when we have decided to use this way of mitigating the SP issue) and then the only remaining one is the ResumeAfterCatch. It seems that one can be modified to use FP as you suggested since we compile with -fno-omit-frame-pointer.

So it seems we can move to using FP instead and get rid of this passing via structs.

@janvorli

Copy link
Copy Markdown
Member

@clamp03 let me try the change that @BrzVlad suggested. If that works, we can close this PR as it would not be needed.

@jkotas

Copy link
Copy Markdown
Member

we compile with -fno-omit-frame-pointer.

That's fragile in a different way. -fno-omit-frame-pointer is not a thing on Windows x64, and the optimizations can setup the FP in non-obvious ways.

@janvorli

Copy link
Copy Markdown
Member

@jkotas on Windows, you are right. We can keep using the SP. On Unix, it seems that it is required for the compiler to not to reuse FP for anything else when -fno-omit-frame-pointer is specified and the function in question is not inlined in some other that is compiled without the -fno-omit-frame-pointer. Both LLVM and GCC marks the frame register register as reserved in such case. Copilot has provided me with links to the source locations in LLVM/GCC and also to related abi docs that seem to confirm that.

However, yesterday I have resumed work on changing the ResumeAfterCatch to get rid of the explicit native frames unwinding and using C++ exception like we do on WASM. That would remove the need for storing the SP or FP completely.

@jakobbotsch

Copy link
Copy Markdown
Member

What is the status here? Did you land the change mentioned above @janvorli?

@janvorli

Copy link
Copy Markdown
Member

@jakobbotsch the change is in PR #128728, I am still fixing some issues with it.

@clamp03

Copy link
Copy Markdown
MemberAuthor

I will close this PR once PR #128728 is merged.

@JulieLeeMSFT
JulieLeeMSFT marked this pull request as draft June 2, 2026 18:11
@janvorli

Copy link
Copy Markdown
Member

@clamp03 I've just merged my PR in.

@clamp03

clamp03 commented Jun 9, 2026

Copy link
Copy Markdown
MemberAuthor

@clamp03 I've just merged my PR in.

Thank you so much!

@janvorli + I will check interpreter on arm32 and then revert all struct parameters in interpexec.

@clamp03clamp03 closed this Jun 9, 2026
@janvorli

Copy link
Copy Markdown
Member

Thank you @clamp03!

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

Labels

arch-arm32area-CodeGen-Interpreter-coreclrcommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

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

[clr-interp] Use struct parameter for InvokeUnmanagedCalliWithTransition - #127543

Closed
clamp03 wants to merge 4 commits into
dotnet:mainfrom
clamp03:bundle_args
Closed

[clr-interp] Use struct parameter for InvokeUnmanagedCalliWithTransition#127543
clamp03 wants to merge 4 commits into
dotnet:mainfrom
clamp03:bundle_args

Conversation

@clamp03

Copy link
Copy Markdown
Member

On ARM32, bundling arguments in a struct forces register passing, ensuring the SP value saved by SAVE_THE_LOWEST_SP matches what stack walking reports.

On ARM32, bundling arguments in a struct forces register passing, ensuring
the SP value saved by SAVE_THE_LOWEST_SP matches what stack walking reports.
@clamp03clamp03 self-assigned this Apr 29, 2026
@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Apr 29, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @BrzVlad, @janvorli, @kg
See info in area-owners.md if you want to be subscribed.

@am11

am11 commented Apr 29, 2026

Copy link
Copy Markdown
Member

@clamp03, if you get a chance, could you also run interpreter tests on riscv64? If something fails, please file an issue with build and test commands. I will take from there (I was struggling to figure out commands with --bootstrap earlier but code was ported to riscv64 to the extent that 'it just compiles' 😅)

@clamp03

Copy link
Copy Markdown
MemberAuthor

@clamp03, if you get a chance, could you also run interpreter tests on riscv64? If something fails, please file an issue with build and test commands. I will take from there (I was struggling to figure out commands with --bootstrap earlier but code was ported to riscv64 to the extent that 'it just compiles' 😅)

@am11 Okay, I'll check the interpreter tests on riscv64 after May 5th, as we have a holiday until then. Since I don't have an available riscv64 device now, I'll run the tests using QEMU. Thanks!

@clamp03

Copy link
Copy Markdown
MemberAuthor

@BrzVlad@davidwrighton@janvorli Could you please review this PR? Thank you.

@BrzVlad

Copy link
Copy Markdown
Member

This seems fragile. Wouldn't it be better to save the FP instead and identify the native frame by this, given this wouldn't change and we wouldn't have to bother with updating every time sp changes ? @janvorli ?

@janvorli

Copy link
Copy Markdown
Member

This seems fragile. Wouldn't it be better to save the FP instead and identify the native frame by this, given this wouldn't change and we wouldn't have to bother with updating every time sp changes ? @janvorli ?

@BrzVlad we did that already for other cases, so this just adds another one. I agree it is ugly. Looking again at where we use the saved SP in the current state, there is one place where we pass it into a call that ends up ignoring it (which wasn't the case when we have decided to use this way of mitigating the SP issue) and then the only remaining one is the ResumeAfterCatch. It seems that one can be modified to use FP as you suggested since we compile with -fno-omit-frame-pointer.

So it seems we can move to using FP instead and get rid of this passing via structs.

@janvorli

Copy link
Copy Markdown
Member

@clamp03 let me try the change that @BrzVlad suggested. If that works, we can close this PR as it would not be needed.

@jkotas

Copy link
Copy Markdown
Member

we compile with -fno-omit-frame-pointer.

That's fragile in a different way. -fno-omit-frame-pointer is not a thing on Windows x64, and the optimizations can setup the FP in non-obvious ways.

@janvorli

Copy link
Copy Markdown
Member

@jkotas on Windows, you are right. We can keep using the SP. On Unix, it seems that it is required for the compiler to not to reuse FP for anything else when -fno-omit-frame-pointer is specified and the function in question is not inlined in some other that is compiled without the -fno-omit-frame-pointer. Both LLVM and GCC marks the frame register register as reserved in such case. Copilot has provided me with links to the source locations in LLVM/GCC and also to related abi docs that seem to confirm that.

However, yesterday I have resumed work on changing the ResumeAfterCatch to get rid of the explicit native frames unwinding and using C++ exception like we do on WASM. That would remove the need for storing the SP or FP completely.

@jakobbotsch

Copy link
Copy Markdown
Member

What is the status here? Did you land the change mentioned above @janvorli?

@janvorli

Copy link
Copy Markdown
Member

@jakobbotsch the change is in PR #128728, I am still fixing some issues with it.

@clamp03

Copy link
Copy Markdown
MemberAuthor

I will close this PR once PR #128728 is merged.

@JulieLeeMSFT
JulieLeeMSFT marked this pull request as draft June 2, 2026 18:11
@janvorli

Copy link
Copy Markdown
Member

@clamp03 I've just merged my PR in.

@clamp03

clamp03 commented Jun 9, 2026

Copy link
Copy Markdown
MemberAuthor

@clamp03 I've just merged my PR in.

Thank you so much!

@janvorli + I will check interpreter on arm32 and then revert all struct parameters in interpexec.

@clamp03clamp03 closed this Jun 9, 2026
@janvorli

Copy link
Copy Markdown
Member

Thank you @clamp03!

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

Labels

arch-arm32area-CodeGen-Interpreter-coreclrcommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

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

[clr-interp] Use struct parameter for InvokeUnmanagedCalliWithTransition - #127543

Closed
clamp03 wants to merge 4 commits into
dotnet:mainfrom
clamp03:bundle_args
Closed

[clr-interp] Use struct parameter for InvokeUnmanagedCalliWithTransition#127543
clamp03 wants to merge 4 commits into
dotnet:mainfrom
clamp03:bundle_args

Conversation

@clamp03

Copy link
Copy Markdown
Member

On ARM32, bundling arguments in a struct forces register passing, ensuring the SP value saved by SAVE_THE_LOWEST_SP matches what stack walking reports.

On ARM32, bundling arguments in a struct forces register passing, ensuring
the SP value saved by SAVE_THE_LOWEST_SP matches what stack walking reports.
@clamp03clamp03 self-assigned this Apr 29, 2026
@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Apr 29, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @BrzVlad, @janvorli, @kg
See info in area-owners.md if you want to be subscribed.

@am11

am11 commented Apr 29, 2026

Copy link
Copy Markdown
Member

@clamp03, if you get a chance, could you also run interpreter tests on riscv64? If something fails, please file an issue with build and test commands. I will take from there (I was struggling to figure out commands with --bootstrap earlier but code was ported to riscv64 to the extent that 'it just compiles' 😅)

@clamp03

Copy link
Copy Markdown
MemberAuthor

@clamp03, if you get a chance, could you also run interpreter tests on riscv64? If something fails, please file an issue with build and test commands. I will take from there (I was struggling to figure out commands with --bootstrap earlier but code was ported to riscv64 to the extent that 'it just compiles' 😅)

@am11 Okay, I'll check the interpreter tests on riscv64 after May 5th, as we have a holiday until then. Since I don't have an available riscv64 device now, I'll run the tests using QEMU. Thanks!

@clamp03

Copy link
Copy Markdown
MemberAuthor

@BrzVlad@davidwrighton@janvorli Could you please review this PR? Thank you.

@BrzVlad

Copy link
Copy Markdown
Member

This seems fragile. Wouldn't it be better to save the FP instead and identify the native frame by this, given this wouldn't change and we wouldn't have to bother with updating every time sp changes ? @janvorli ?

@janvorli

Copy link
Copy Markdown
Member

This seems fragile. Wouldn't it be better to save the FP instead and identify the native frame by this, given this wouldn't change and we wouldn't have to bother with updating every time sp changes ? @janvorli ?

@BrzVlad we did that already for other cases, so this just adds another one. I agree it is ugly. Looking again at where we use the saved SP in the current state, there is one place where we pass it into a call that ends up ignoring it (which wasn't the case when we have decided to use this way of mitigating the SP issue) and then the only remaining one is the ResumeAfterCatch. It seems that one can be modified to use FP as you suggested since we compile with -fno-omit-frame-pointer.

So it seems we can move to using FP instead and get rid of this passing via structs.

@janvorli

Copy link
Copy Markdown
Member

@clamp03 let me try the change that @BrzVlad suggested. If that works, we can close this PR as it would not be needed.

@jkotas

Copy link
Copy Markdown
Member

we compile with -fno-omit-frame-pointer.

That's fragile in a different way. -fno-omit-frame-pointer is not a thing on Windows x64, and the optimizations can setup the FP in non-obvious ways.

@janvorli

Copy link
Copy Markdown
Member

@jkotas on Windows, you are right. We can keep using the SP. On Unix, it seems that it is required for the compiler to not to reuse FP for anything else when -fno-omit-frame-pointer is specified and the function in question is not inlined in some other that is compiled without the -fno-omit-frame-pointer. Both LLVM and GCC marks the frame register register as reserved in such case. Copilot has provided me with links to the source locations in LLVM/GCC and also to related abi docs that seem to confirm that.

However, yesterday I have resumed work on changing the ResumeAfterCatch to get rid of the explicit native frames unwinding and using C++ exception like we do on WASM. That would remove the need for storing the SP or FP completely.

@jakobbotsch

Copy link
Copy Markdown
Member

What is the status here? Did you land the change mentioned above @janvorli?

@janvorli

Copy link
Copy Markdown
Member

@jakobbotsch the change is in PR #128728, I am still fixing some issues with it.

@clamp03

Copy link
Copy Markdown
MemberAuthor

I will close this PR once PR #128728 is merged.

@JulieLeeMSFT
JulieLeeMSFT marked this pull request as draft June 2, 2026 18:11
@janvorli

Copy link
Copy Markdown
Member

@clamp03 I've just merged my PR in.

@clamp03

clamp03 commented Jun 9, 2026

Copy link
Copy Markdown
MemberAuthor

@clamp03 I've just merged my PR in.

Thank you so much!

@janvorli + I will check interpreter on arm32 and then revert all struct parameters in interpexec.

@clamp03clamp03 closed this Jun 9, 2026
@janvorli

Copy link
Copy Markdown
Member

Thank you @clamp03!

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

Labels

arch-arm32area-CodeGen-Interpreter-coreclrcommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

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

[clr-interp] Use struct parameter for InvokeUnmanagedCalliWithTransition - #127543

Closed
clamp03 wants to merge 4 commits into
dotnet:mainfrom
clamp03:bundle_args
Closed

[clr-interp] Use struct parameter for InvokeUnmanagedCalliWithTransition#127543
clamp03 wants to merge 4 commits into
dotnet:mainfrom
clamp03:bundle_args

Conversation

@clamp03

Copy link
Copy Markdown
Member

On ARM32, bundling arguments in a struct forces register passing, ensuring the SP value saved by SAVE_THE_LOWEST_SP matches what stack walking reports.

On ARM32, bundling arguments in a struct forces register passing, ensuring
the SP value saved by SAVE_THE_LOWEST_SP matches what stack walking reports.
@clamp03clamp03 self-assigned this Apr 29, 2026
@dotnet-policy-servicedotnet-policy-serviceBot added the community-contribution Indicates that the PR has been added by a community member label Apr 29, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @BrzVlad, @janvorli, @kg
See info in area-owners.md if you want to be subscribed.

@am11

am11 commented Apr 29, 2026

Copy link
Copy Markdown
Member

@clamp03, if you get a chance, could you also run interpreter tests on riscv64? If something fails, please file an issue with build and test commands. I will take from there (I was struggling to figure out commands with --bootstrap earlier but code was ported to riscv64 to the extent that 'it just compiles' 😅)

@clamp03

Copy link
Copy Markdown
MemberAuthor

@clamp03, if you get a chance, could you also run interpreter tests on riscv64? If something fails, please file an issue with build and test commands. I will take from there (I was struggling to figure out commands with --bootstrap earlier but code was ported to riscv64 to the extent that 'it just compiles' 😅)

@am11 Okay, I'll check the interpreter tests on riscv64 after May 5th, as we have a holiday until then. Since I don't have an available riscv64 device now, I'll run the tests using QEMU. Thanks!

@clamp03

Copy link
Copy Markdown
MemberAuthor

@BrzVlad@davidwrighton@janvorli Could you please review this PR? Thank you.

@BrzVlad

Copy link
Copy Markdown
Member

This seems fragile. Wouldn't it be better to save the FP instead and identify the native frame by this, given this wouldn't change and we wouldn't have to bother with updating every time sp changes ? @janvorli ?

@janvorli

Copy link
Copy Markdown
Member

This seems fragile. Wouldn't it be better to save the FP instead and identify the native frame by this, given this wouldn't change and we wouldn't have to bother with updating every time sp changes ? @janvorli ?

@BrzVlad we did that already for other cases, so this just adds another one. I agree it is ugly. Looking again at where we use the saved SP in the current state, there is one place where we pass it into a call that ends up ignoring it (which wasn't the case when we have decided to use this way of mitigating the SP issue) and then the only remaining one is the ResumeAfterCatch. It seems that one can be modified to use FP as you suggested since we compile with -fno-omit-frame-pointer.

So it seems we can move to using FP instead and get rid of this passing via structs.

@janvorli

Copy link
Copy Markdown
Member

@clamp03 let me try the change that @BrzVlad suggested. If that works, we can close this PR as it would not be needed.

@jkotas

Copy link
Copy Markdown
Member

we compile with -fno-omit-frame-pointer.

That's fragile in a different way. -fno-omit-frame-pointer is not a thing on Windows x64, and the optimizations can setup the FP in non-obvious ways.

@janvorli

Copy link
Copy Markdown
Member

@jkotas on Windows, you are right. We can keep using the SP. On Unix, it seems that it is required for the compiler to not to reuse FP for anything else when -fno-omit-frame-pointer is specified and the function in question is not inlined in some other that is compiled without the -fno-omit-frame-pointer. Both LLVM and GCC marks the frame register register as reserved in such case. Copilot has provided me with links to the source locations in LLVM/GCC and also to related abi docs that seem to confirm that.

However, yesterday I have resumed work on changing the ResumeAfterCatch to get rid of the explicit native frames unwinding and using C++ exception like we do on WASM. That would remove the need for storing the SP or FP completely.

@jakobbotsch

Copy link
Copy Markdown
Member

What is the status here? Did you land the change mentioned above @janvorli?

@janvorli

Copy link
Copy Markdown
Member

@jakobbotsch the change is in PR #128728, I am still fixing some issues with it.

@clamp03

Copy link
Copy Markdown
MemberAuthor

I will close this PR once PR #128728 is merged.

@JulieLeeMSFT
JulieLeeMSFT marked this pull request as draft June 2, 2026 18:11
@janvorli

Copy link
Copy Markdown
Member

@clamp03 I've just merged my PR in.

@clamp03

clamp03 commented Jun 9, 2026

Copy link
Copy Markdown
MemberAuthor

@clamp03 I've just merged my PR in.

Thank you so much!

@janvorli + I will check interpreter on arm32 and then revert all struct parameters in interpexec.

@clamp03clamp03 closed this Jun 9, 2026
@janvorli

Copy link
Copy Markdown
Member

Thank you @clamp03!

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

Labels

arch-arm32area-CodeGen-Interpreter-coreclrcommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@clamp03@am11@BrzVlad@janvorli@jkotas@jakobbotsch