Skip to content
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

Refactor - removes usage of REGISTER_OTHER_SCRATCH #597

Merged
merged 5 commits into from
Sep 27, 2024

Conversation

Lichtso
Copy link

@Lichtso Lichtso commented Sep 26, 2024

Increases the machinecode emitted for:

  • store immediate from 28 to 36 (+29%)
  • store register from 24 to 29 (+21%)

@Lichtso Lichtso force-pushed the refactor/argument_register_indirect_rsp branch from 350d954 to 0628d68 Compare September 26, 2024 13:19
@Lichtso Lichtso requested a review from LucasSte September 26, 2024 14:58
@@ -1554,7 +1556,7 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> {
};
self.emit_rust_call(Value::Constant64(store, false), &[
Argument { index: 3, value: Value::Register(REGISTER_SCRATCH) }, // Specify first as the src register could be overwritten by other arguments
Argument { index: 2, value: Value::Register(REGISTER_OTHER_SCRATCH) },
Argument { index: 2, value: Value::RegisterIndirect(RSP, -8, false) },

Choose a reason for hiding this comment

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

Shouldn't the offset for stack_slot_of_value_to_store (i.e. -112) be the same as the one here (-8)?

Copy link
Author

@Lichtso Lichtso Sep 26, 2024

Choose a reason for hiding this comment

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

At this point the start of emit_rust_call has already saved all the VM register state, thus the stack is taller. RSP-8 is pointing to the next stack slot to be pushed. This is where we temporarily saved the "value to store" at. It will be overwritten by the RIP in the subsequent call in the end of emit_rust_call.

@Lichtso Lichtso force-pushed the refactor/argument_register_indirect_rsp branch from 0628d68 to 8c5e9a9 Compare September 26, 2024 21:14
@Lichtso Lichtso merged commit 7364447 into main Sep 27, 2024
12 checks passed
@Lichtso Lichtso deleted the refactor/argument_register_indirect_rsp branch September 27, 2024 18:09
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.

2 participants