Skip to content

windows: clobber nonvolatile regs#983

Closed
dozreg-toplud wants to merge 3 commits intodevelopfrom
dozreg/windows-setjmp
Closed

windows: clobber nonvolatile regs#983
dozreg-toplud wants to merge 3 commits intodevelopfrom
dozreg/windows-setjmp

Conversation

@dozreg-toplud
Copy link
Copy Markdown
Contributor

@dozreg-toplud dozreg-toplud commented Mar 23, 2026

Per docs:

... this intrinsic forces register saving for the current function...

Maybe we should still call the builtin just to signal to the compiler that the registers ought to be saved?

@dozreg-toplud dozreg-toplud requested a review from pkova March 23, 2026 18:27
@dozreg-toplud dozreg-toplud requested a review from a team as a code owner March 23, 2026 18:27
@dozreg-toplud
Copy link
Copy Markdown
Contributor Author

Notably the resumption is in windows_setjmp now instead of its caller. Does it matter?

@dozreg-toplud
Copy link
Copy Markdown
Contributor Author

In testing both your and my branches kill the process on !! in Dojo...

@dozreg-toplud dozreg-toplud force-pushed the dozreg/windows-setjmp branch from 3dcafb7 to 814089b Compare March 24, 2026 10:02
@dozreg-toplud
Copy link
Copy Markdown
Contributor Author

dozreg-toplud commented Mar 24, 2026

Both branches were killing the process with optimizations enabled, on debug both worked. That led me to believe it's the register allocation.

I tried calling the intrinsic with the goal of clobbering all registers, which did not work for some reason. Then I added manual clobbering of all callee-saved registers (caller-saved registers are already clobbered by calling windows_setjmp, which I force to not be inlined), and it did work on ReleaseFast

@dozreg-toplud dozreg-toplud changed the title windows: still call the intinsic windows: clobber nonvolatile regs Mar 24, 2026
@dozreg-toplud
Copy link
Copy Markdown
Contributor Author

dozreg-toplud commented Mar 24, 2026

Added register clobbering after longjmp as well as before setjmp. Tested on vere64, seems to work

@dozreg-toplud
Copy link
Copy Markdown
Contributor Author

This is rendered obsolete by #987

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant