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

YJIT: Inline simple getlocal+leave iseqs #10360

Merged
merged 1 commit into from Mar 25, 2024
Merged

Conversation

XrXr
Copy link
Member

@XrXr XrXr commented Mar 25, 2024

This mainly targets things like T.let() from Sorbet, which is just an
identity function at runtime and only a hint for the static checker.
Only deal with simple caller and callees (no keywords and splat etc.).

@XrXr XrXr requested a review from a team as a code owner March 25, 2024 20:18
yjit/src/codegen.rs Outdated Show resolved Hide resolved
yjit/src/codegen.rs Outdated Show resolved Hide resolved
@@ -6774,6 +6779,17 @@ fn iseq_get_return_value(iseq: IseqPtr, captured_opnd: Option<Opnd>) -> Option<I
return None;
}
match first_insn {
YARVINSN_getlocal_WC_0 => {
// Only accept simple positional only cases for both the caller and the callee.
// Reject block ISEQs to avoid autosplat and other block parameter complications.
Copy link
Member

Choose a reason for hiding this comment

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

Rejecting the captured_opnd.is_some() case only impacts yield. Is this not a problem for block.call?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is a problem for block.call, but we essentially do a manual fallback with rb_optimized_call() for those cases 😅 All the block setup code check captured_opnd .

@jez
Copy link
Contributor

jez commented Mar 25, 2024

This mainly targets things like T.let() from Sorbet

I think possibly you mean T.unsafe, unless you're also working with a monkey patch of T.let defined like this:

def self.let(x, type)
  x
end

anyways, neat PR!

This mainly targets things like `T.unsafe()` from Sorbet, which is just an
identity function at runtime and only a hint for the static checker.
Only deal with simple caller and callees (no keywords and splat etc.).

Co-authored-by: Takashi Kokubun (k0kubun) <takashikkbn@gmail.com>
@XrXr XrXr enabled auto-merge (rebase) March 25, 2024 21:27
@XrXr XrXr merged commit de742b4 into ruby:master Mar 25, 2024
98 checks passed
@XrXr XrXr deleted the yjit-inline-identity branch March 25, 2024 21:50
@casperisfine
Copy link
Contributor

you're also working with a monkey patch of T.let defined like this:

We do yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants