Implement getspecial in ZJIT #13642

eileencodes · 2025-06-17T17:59:46Z

Adds support for the getspecial instruction in zjit.

We split getspecial into two instructions, one for special symbols ($&, $', etc) and one for special backrefs ($1, $2`, etc).

launchable-app · 2025-06-17T18:39:16Z

❌ Tests Failed

✖️no tests failed ✔️62025 tests passed(1 flake)

tekknolagi · 2025-06-18T05:10:03Z

zjit/src/codegen.rs

+            asm_comment!(asm, "rb_reg_match_last");
+            asm.ccall(rb_reg_match_last as *const u8, vec![backref])
+        }
+        _ => panic!("invalid back-ref"),


Suggested change

_ => panic!("invalid back-ref"),

c => panic!("invalid back-ref {c}"),

If you end up making an enum, it would be great to shift this into the parser into HIR so that we have more up-front guarantees of program legality

tekknolagi · 2025-06-18T05:11:16Z

zjit/src/hir.rs

+    GetSpecialSymbol { key: u64, svar: u64, state: InsnId },
+    GetSpecialNumber { key: u64, svar: u64, state: InsnId },


Let's store the svar as a new enum -- it seems to not make full use of the u64 and we can have nicer looking code this way

tekknolagi · 2025-06-18T05:12:35Z

zjit/src/hir.rs

@@ -446,6 +446,8 @@ pub enum Insn {
    /// Check whether an instance variable exists on `self_val`
    DefinedIvar { self_val: InsnId, id: ID, pushval: VALUE, state: InsnId },

+    GetSpecialSymbol { key: u64, svar: u64, state: InsnId },


Can we also make key an enum? LastLine | BackRef | Offset(n)

Unless this makes code generation notably uglier

tekknolagi · 2025-06-18T05:14:44Z

zjit/src/codegen.rs

@@ -379,6 +381,53 @@ fn gen_putspecialobject(asm: &mut Assembler, value_type: SpecialObjectType) -> O
    )
 }

+fn gen_getspecial_symbol(asm: &mut Assembler, _key: u64, svar: u64) -> Opnd {


Why don't we use key?

tekknolagi · 2025-06-18T05:16:01Z

zjit/src/hir.rs

+                    if svar == 0 {
+                        let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state });
+                        fun.push_insn(block, Insn::SideExit { state: exit_id });
+                    } else if svar & 0x01 != 0 {
+                        // Handle symbol backrefs like $&, $`, $\, $+
+                        let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state });
+                        let result = fun.push_insn(block, Insn::GetSpecialSymbol { key, svar, state: exit_id });
+                        state.stack_push(result);
+                    } else {
+                        // Handle number backrefs like $1, $2, $3
+                        let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state });
+                        let result = fun.push_insn(block, Insn::GetSpecialNumber { key, svar, state: exit_id });
+                        state.stack_push(result);
+                    }


Suggested change

if svar == 0 {

let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state });

fun.push_insn(block, Insn::SideExit { state: exit_id });

} else if svar & 0x01 != 0 {

// Handle symbol backrefs like $&, $`, $\, $+

let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state });

let result = fun.push_insn(block, Insn::GetSpecialSymbol { key, svar, state: exit_id });

state.stack_push(result);

} else {

// Handle number backrefs like $1, $2, $3

let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state });

let result = fun.push_insn(block, Insn::GetSpecialNumber { key, svar, state: exit_id });

state.stack_push(result);

}

let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state });

if svar == 0 {

// TODO: Handle whatever this is

fun.push_insn(block, Insn::SideExit { state: exit_id });

} else if svar & 0x01 != 0 {

// Handle symbol backrefs like $&, $`, $\, $+

let result = fun.push_insn(block, Insn::GetSpecialSymbol { key, svar, state: exit_id });

state.stack_push(result);

} else {

// Handle number backrefs like $1, $2, $3

let result = fun.push_insn(block, Insn::GetSpecialNumber { key, svar, state: exit_id });

state.stack_push(result);

}

tekknolagi · 2025-06-18T05:16:42Z

zjit/src/codegen.rs

+        vec![
+        Opnd::Imm((svar >> 1).try_into().unwrap()),
+        backref,
+        ]


Suggested change

vec![

Opnd::Imm((svar >> 1).try_into().unwrap()),

backref,

]

vec![

Opnd::Imm((svar >> 1).try_into().unwrap()),

backref,

]

Why 2 spaces and not 4? Can we use rustfmt? Or vim indentation rules?

I meant use any indentation at all. For better or worse, we don't (currently) use a formatter

tekknolagi · 2025-06-18T05:18:16Z

zjit/src/codegen.rs

@@ -379,6 +381,53 @@ fn gen_putspecialobject(asm: &mut Assembler, value_type: SpecialObjectType) -> O
    )
 }

+fn gen_getspecial_symbol(asm: &mut Assembler, _key: u64, svar: u64) -> Opnd {


I think you need to save the PC for some of these because they can allocate or raise

Adds support for the getspecial instruction in zjit. We split getspecial into two instructions, one for special symbols (`$&`, $'`, etc) and one for special backrefs (`$1`, `$2`, etc). Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>

eileencodes · 2025-06-27T17:43:08Z

Ok I think I fixed the things you asked for, happy to make more changes though if I missed anything.

eileencodes · 2025-06-27T18:03:26Z

Nope I need to fix the failures 🤦🏼‍♀️

tekknolagi

This looks great -- all I really have is small comments, which if you are short on time I can do in a follow-up

tekknolagi · 2025-06-27T17:56:51Z

zjit/src/codegen.rs

        Insn::SideExit { state } => return gen_side_exit(jit, asm, &function.frame_state(*state)),
        Insn::PutSpecialObject { value_type } => gen_putspecialobject(asm, *value_type),
        Insn::AnyToString { val, str, state } => gen_anytostring(asm, opnd!(val), opnd!(str), &function.frame_state(*state))?,
        Insn::Defined { op_type, obj, pushval, v } => gen_defined(jit, asm, *op_type, *obj, *pushval, opnd!(v))?,
+        Insn::GetSpecialSymbol { symbol_type, state: _ } => { gen_getspecial_symbol(asm, 0, *symbol_type) },


Suggested change

Insn::GetSpecialSymbol { symbol_type, state: _ } => { gen_getspecial_symbol(asm, 0, *symbol_type) },

Insn::GetSpecialSymbol { symbol_type, state: _ } => gen_getspecial_symbol(asm, 0, *symbol_type),

tekknolagi · 2025-06-27T17:57:27Z

zjit/src/codegen.rs

@@ -278,16 +278,18 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
        Insn::PatchPoint(_) => return Some(()), // For now, rb_zjit_bop_redefined() panics. TODO: leave a patch point and fix rb_zjit_bop_redefined()
        Insn::CCall { cfun, args, name: _, return_type: _, elidable: _ } => gen_ccall(jit, asm, *cfun, args)?,
        Insn::GetIvar { self_val, id, state: _ } => gen_getivar(asm, opnd!(self_val), *id),
+        Insn::SetIvar { self_val, id, val, state: _ } => return gen_setivar(asm, opnd!(self_val), *id, opnd!(val)),


nit: please split this out into a separate commit (in an ideal world we minimize changes per commit and keep them grouped)

tekknolagi · 2025-06-27T17:57:50Z

zjit/src/codegen.rs

+    // call rb_backref_get()
+    asm_comment!(asm, "rb_backref_get");


Suggested change

// call rb_backref_get()

asm_comment!(asm, "rb_backref_get");

asm_comment!(asm, "call rb_backref_get");

(here and for all ccalls)

tekknolagi · 2025-06-27T18:00:22Z

zjit/src/hir.rs

+                    if svar == 0 {
+                        fun.push_insn(block, Insn::SideExit { state: exit_id });


Suggested change

if svar == 0 {

fun.push_insn(block, Insn::SideExit { state: exit_id });

if svar == 0 {

// TODO: Handle non-backref

fun.push_insn(block, Insn::SideExit { state: exit_id });

(or whatever this thing is called)

tekknolagi · 2025-06-27T18:03:01Z

zjit/src/hir.rs

@@ -452,6 +475,8 @@ pub enum Insn {
    GetLocal { level: NonZeroU32, ep_offset: u32 },
    /// Set a local variable in a higher scope
    SetLocal { level: NonZeroU32, ep_offset: u32, val: InsnId },
+    GetSpecialSymbol { symbol_type: SpecialBackrefSymbol, state: InsnId },
+    GetSpecialNumber { svar: u64, state: InsnId },


Suggested change

GetSpecialNumber { svar: u64, state: InsnId },

GetSpecialNumber { nth: u64, state: InsnId },

This is called nth internally that seems like it communicates more intent

tekknolagi · 2025-06-27T18:03:17Z

zjit/src/hir.rs

+                        state.stack_push(result);
+                    } else {
+                        // Handle number backrefs like $1, $2, $3
+                        let result = fun.push_insn(block, Insn::GetSpecialNumber { svar, state: exit_id });


Suggested change

let result = fun.push_insn(block, Insn::GetSpecialNumber { svar, state: exit_id });

let result = fun.push_insn(block, Insn::GetSpecialNumber { nth: (svar>>1), state: exit_id });

tekknolagi · 2025-06-27T18:11:16Z

zjit/src/codegen.rs

+    let backref = asm.ccall(rb_backref_get as *const u8, vec![]);
+
+    // rb_reg_nth_match((int)(type >> 1), backref);
+    asm_comment!(asm, "rb_reg_nth_match");
+    asm.ccall(


Suggested change

let backref = asm.ccall(rb_backref_get as *const u8, vec![]);

// rb_reg_nth_match((int)(type >> 1), backref);

asm_comment!(asm, "rb_reg_nth_match");

asm.ccall(

let backref = asm.ccall(rb_backref_get as *const u8, vec![]);

// Save PC for GC

gen_save_pc(asm, state);

// rb_reg_nth_match((int)(type >> 1), backref);

asm_comment!(asm, "rb_reg_nth_match");

asm.ccall(

Sorry about indentation, but since this can allocate (str substr), we should save the PC for more accurate allocation tracing. Here and above

matzbot requested a review from a team June 17, 2025 17:59

eileencodes force-pushed the zjit-implement-getspecial branch from a608c69 to 06d112a Compare June 17, 2025 18:15

tekknolagi reviewed Jun 18, 2025

View reviewed changes

Implement getspecial in ZJIT

040e847

Adds support for the getspecial instruction in zjit. We split getspecial into two instructions, one for special symbols (`$&`, $'`, etc) and one for special backrefs (`$1`, `$2`, etc). Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>

eileencodes force-pushed the zjit-implement-getspecial branch from 06d112a to 040e847 Compare June 27, 2025 17:41

tekknolagi approved these changes Jun 27, 2025

View reviewed changes

tekknolagi reviewed Jun 27, 2025

View reviewed changes

	_ => panic!("invalid back-ref"),
	c => panic!("invalid back-ref {c}"),

		GetSpecialSymbol { key: u64, svar: u64, state: InsnId },
		GetSpecialNumber { key: u64, svar: u64, state: InsnId },

	Insn::GetSpecialSymbol { symbol_type, state: _ } => { gen_getspecial_symbol(asm, 0, *symbol_type) },
	Insn::GetSpecialSymbol { symbol_type, state: _ } => gen_getspecial_symbol(asm, 0, *symbol_type),

		// call rb_backref_get()
		asm_comment!(asm, "rb_backref_get");

	// call rb_backref_get()
	asm_comment!(asm, "rb_backref_get");
	asm_comment!(asm, "call rb_backref_get");

		if svar == 0 {
		fun.push_insn(block, Insn::SideExit { state: exit_id });

	GetSpecialNumber { svar: u64, state: InsnId },
	GetSpecialNumber { nth: u64, state: InsnId },

	let result = fun.push_insn(block, Insn::GetSpecialNumber { svar, state: exit_id });
	let result = fun.push_insn(block, Insn::GetSpecialNumber { nth: (svar>>1), state: exit_id });

Implement getspecial in ZJIT #13642

Are you sure you want to change the base?

Implement getspecial in ZJIT #13642

Uh oh!

Conversation

eileencodes commented Jun 17, 2025

Uh oh!

launchable-app bot commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ Tests Failed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eileencodes commented Jun 27, 2025

Uh oh!

eileencodes commented Jun 27, 2025

Uh oh!

tekknolagi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

launchable-app bot commented Jun 17, 2025 •

edited

Loading