New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
YJIT: Use .as_side_exit() for jumps to counted exits #7150
Conversation
Fewer cycles running nops when these jumps are not taken. Fixing all these so when they get copy pasted in the future we save on padding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Thank you Alan! |
Why make it so we have to ensure we have as_side_exit rather than fixing the api to make this mistake difficult/impossible? Why the CodePtr/Target split for this stuff? Do we really want to default to unnamed side_exits? We have a lot of code we have to copy and paste to ensure we do things properly. Seems like with a little thought and some nice data types, we could avoid all of that. |
We have a
Presumably, when we generate a side-exit, it should always have the side exit type 😅 |
So, I also considered another change where I do: M yjit/src/codegen.rs
@@ -201,7 +201,7 @@ macro_rules! counted_exit {
($ocb:tt, $existing_side_exit:tt, $counter_name:ident) => {
// The counter is only incremented when stats are enabled
if (!get_option!(gen_stats)) {
- $existing_side_exit
+ $existing_side_exit.as_side_exit()
} else {
let ocb = $ocb.unwrap();
let code_ptr = ocb.get_write_ptr();
@@ -216,7 +216,7 @@ macro_rules! counted_exit {
ocb_asm.compile(ocb);
// Pointer to the side-exit code
- code_ptr
+ code_ptr.as_side_exit()
}
};
} that one gave me 17 compile errors versus this diff which gave me under 10 lines to change. |
I'll make a PR with the change. But going to wait till #6929 is merged so I don't have to deal with resolving that conflict |
Fewer cycles running nops when these jumps are not taken. Fixing all
these so when they get copy pasted in the future we save on padding.