Skip to content

Commit a8ef184

Browse files
committed
ZJIT: Implement concatstrings
Quite tricky to use the native stack to pass a C `VALUE[]`.
1 parent 424485d commit a8ef184

6 files changed

Lines changed: 85 additions & 9 deletions

File tree

test/ruby/test_zjit.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,13 @@ def test = "#{""}"
3838
}, insns: [:putstring]
3939
end
4040

41+
def test_concatstrings
42+
assert_compiles '"Object is not Kernel"', %q{
43+
def test = "#{Object} is not #{Kernel}"
44+
test
45+
}, insns: [:concatstrings]
46+
end
47+
4148
def test_putchilldedstring
4249
assert_compiles '""', %q{
4350
def test = ""

zjit/src/backend/arm64/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ pub const _C_RET_OPND: Opnd = Opnd::Reg(X0_REG);
3232
// These constants define the way we work with Arm64's stack pointer. The stack
3333
// pointer always needs to be aligned to a 16-byte boundary.
3434
pub const C_SP_REG: A64Opnd = X31;
35+
pub const C_SP_OPND: Opnd = Opnd::Reg(XZR_REG);
3536
pub const C_SP_STEP: i32 = 16;
3637

3738
impl CodeBlock {

zjit/src/backend/lir.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ pub const SP: Opnd = _SP;
1616

1717
pub const C_ARG_OPNDS: [Opnd; 6] = _C_ARG_OPNDS;
1818
pub const C_RET_OPND: Opnd = _C_RET_OPND;
19-
pub use crate::backend::current::{Reg, C_RET_REG};
19+
pub use crate::backend::current::{Reg, C_RET_REG, C_SP_OPND};
2020

2121
// Memory operand base
2222
#[derive(Clone, Copy, PartialEq, Eq, Debug)]

zjit/src/backend/x86_64/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ pub const _C_ARG_OPNDS: [Opnd; 6] = [
2828
// C return value register on this platform
2929
pub const C_RET_REG: Reg = RAX_REG;
3030
pub const _C_RET_OPND: Opnd = Opnd::Reg(RAX_REG);
31+
pub const C_SP_OPND: Opnd = Opnd::Reg(RSP_REG);
3132

3233
impl CodeBlock {
3334
// The number of bytes that are generated by jmp_ptr

zjit/src/codegen.rs

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::backend::current::{Reg, ALLOC_REGS};
66
use crate::profile::get_or_create_iseq_payload;
77
use crate::state::ZJITState;
88
use crate::{asm::CodeBlock, cruby::*, options::debug, virtualmem::CodePtr};
9-
use crate::backend::lir::{self, asm_comment, Assembler, Opnd, Target, CFP, C_ARG_OPNDS, C_RET_OPND, EC, SP};
9+
use crate::backend::lir::{self, asm_comment, Assembler, Opnd, Target, CFP, C_ARG_OPNDS, C_SP_OPND, C_RET_OPND, EC, SP};
1010
use crate::hir::{iseq_to_hir, Block, BlockId, BranchEdge, CallInfo, RangeType, SELF_PARAM_IDX, SpecialObjectType};
1111
use crate::hir::{Const, FrameState, Function, Insn, InsnId};
1212
use crate::hir_type::{types::Fixnum, Type};
@@ -278,6 +278,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
278278
Insn::SideExit { state } => return gen_side_exit(jit, asm, &function.frame_state(*state)),
279279
Insn::PutSpecialObject { value_type } => gen_putspecialobject(asm, *value_type),
280280
Insn::AnyToString { val, str, state } => gen_anytostring(asm, opnd!(val), opnd!(str), &function.frame_state(*state))?,
281+
Insn::ConcatStrings { strings, state } => gen_concatstrings(jit, asm, strings, &function.frame_state(*state))?,
281282
Insn::Defined { op_type, obj, pushval, v } => gen_defined(jit, asm, *op_type, *obj, *pushval, opnd!(v))?,
282283
_ => {
283284
debug!("ZJIT: gen_function: unexpected insn {insn}");
@@ -293,6 +294,48 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
293294
Some(())
294295
}
295296

297+
fn gen_concatstrings(jit: &JITState, asm: &mut Assembler, strings: &[InsnId], state: &FrameState) -> Option<Opnd> {
298+
// In practice, concatstrings with 0 strings doesn't seem to happen.
299+
if strings.len() == 0 {
300+
return None;
301+
}
302+
303+
// Find how much to grow the stack, aligning to 16 for calling convention
304+
let mut stack_growth = strings.len() * SIZEOF_VALUE;
305+
if stack_growth % 16 != 0 {
306+
stack_growth += 16 - stack_growth % 16;
307+
}
308+
309+
// Grow the stack and put the strings onto the stack
310+
for (i, string) in strings.iter().enumerate() {
311+
let dest = Opnd::mem(64, C_SP_OPND, i32::try_from(strings.len() - i).ok()? * -SIZEOF_VALUE_I32);
312+
asm.mov(dest, jit.get_opnd(*string)?);
313+
}
314+
// Write, then move the SP, since the source might be from the stack
315+
let new_sp = asm.sub(C_SP_OPND, stack_growth.into());
316+
asm.mov(C_SP_OPND, new_sp);
317+
318+
// Save PC since the call allocates and can raise
319+
gen_save_pc(asm, state);
320+
321+
// We've moved the SP, so the array is at SP or one element above
322+
// if we grew the stack with extra space for alignment.
323+
let str_array = if strings.len() % 2 == 0 {
324+
C_SP_OPND
325+
} else {
326+
asm.add(C_SP_OPND, SIZEOF_VALUE.into())
327+
};
328+
329+
// Call rb_str_concat_literals()
330+
let new_string = asm.ccall(rb_str_concat_literals as _, vec![strings.len().into(), str_array]);
331+
332+
// Undo C SP change
333+
let prev_sp = asm.add(C_SP_OPND, stack_growth.into());
334+
asm.mov(C_SP_OPND, prev_sp);
335+
336+
Some(new_string)
337+
}
338+
296339
/// Gets the EP of the ISeq of the containing method, or "local level".
297340
/// Equivalent of GET_LEP() macro.
298341
fn gen_get_lep(jit: &JITState, asm: &mut Assembler) -> Opnd {

zjit/src/hir.rs

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,7 @@ pub enum Insn {
507507
// Distinct from `SendWithoutBlock` with `mid:to_s` because does not have a patch point for String to_s being redefined
508508
ObjToString { val: InsnId, call_info: CallInfo, cd: *const rb_call_data, state: InsnId },
509509
AnyToString { val: InsnId, str: InsnId, state: InsnId },
510+
ConcatStrings { strings: Vec<InsnId>, state: InsnId },
510511

511512
/// Side-exit if val doesn't have the expected type.
512513
GuardType { val: InsnId, guard_type: Type, state: InsnId },
@@ -683,7 +684,14 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> {
683684
write!(f, ", {arg}")?;
684685
}
685686
Ok(())
686-
},
687+
}
688+
Insn::ConcatStrings { strings, state: _ } => {
689+
write!(f, "ConcatStrings {}", strings[0])?;
690+
for string in strings.iter().skip(1) {
691+
write!(f, ", {string}")?;
692+
}
693+
Ok(())
694+
}
687695
Insn::Snapshot { state } => write!(f, "Snapshot {}", state),
688696
Insn::Defined { op_type, v, .. } => {
689697
// op_type (enum defined_type) printing logic from iseq.c.
@@ -1066,6 +1074,7 @@ impl Function {
10661074
str: find!(*str),
10671075
state: *state,
10681076
},
1077+
ConcatStrings { state, strings } => ConcatStrings { state: *state, strings: find_vec!(strings) },
10691078
SendWithoutBlock { self_val, call_info, cd, args, state } => SendWithoutBlock {
10701079
self_val: find!(*self_val),
10711080
call_info: call_info.clone(),
@@ -1197,6 +1206,7 @@ impl Function {
11971206
Insn::ToArray { .. } => types::ArrayExact,
11981207
Insn::ObjToString { .. } => types::BasicObject,
11991208
Insn::AnyToString { .. } => types::String,
1209+
Insn::ConcatStrings { .. } => types::StringExact,
12001210
Insn::GetLocal { .. } => types::BasicObject,
12011211
}
12021212
}
@@ -1756,7 +1766,8 @@ impl Function {
17561766
worklist.push_back(state);
17571767
}
17581768
Insn::ArrayMax { elements, state }
1759-
| Insn::NewArray { elements, state } => {
1769+
| Insn::NewArray { elements, state }
1770+
| Insn::ConcatStrings { strings: elements, state } => {
17601771
worklist.extend(elements);
17611772
worklist.push_back(state);
17621773
}
@@ -2917,6 +2928,16 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
29172928
let anytostring = fun.push_insn(block, Insn::AnyToString { val, str, state: exit_id });
29182929
state.stack_push(anytostring);
29192930
}
2931+
YARVINSN_concatstrings => {
2932+
let num = get_arg(pc, 0).as_u32();
2933+
let mut strings = Vec::with_capacity(num.as_usize());
2934+
for _ in 0..num {
2935+
strings.push(state.stack_pop()?);
2936+
}
2937+
strings.reverse();
2938+
let snapshot = fun.push_insn(block, Insn::Snapshot { state: exit_state });
2939+
state.stack_push(fun.push_insn(block, Insn::ConcatStrings { state: snapshot, strings }));
2940+
}
29202941
_ => {
29212942
// Unknown opcode; side-exit into the interpreter
29222943
let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state });
@@ -4642,7 +4663,8 @@ mod tests {
46424663
v3:Fixnum[1] = Const Value(1)
46434664
v5:BasicObject = ObjToString v3
46444665
v7:String = AnyToString v3, str: v5
4645-
SideExit
4666+
v9:StringExact = ConcatStrings v2, v7
4667+
Return v9
46464668
"#]]);
46474669
}
46484670

@@ -6220,7 +6242,8 @@ mod opt_tests {
62206242
v2:StringExact[VALUE(0x1000)] = Const Value(VALUE(0x1000))
62216243
v3:StringExact[VALUE(0x1008)] = Const Value(VALUE(0x1008))
62226244
v4:StringExact = StringCopy v3
6223-
SideExit
6245+
v10:StringExact = ConcatStrings v2, v4
6246+
Return v10
62246247
"#]]);
62256248
}
62266249

@@ -6234,9 +6257,10 @@ mod opt_tests {
62346257
bb0(v0:BasicObject):
62356258
v2:StringExact[VALUE(0x1000)] = Const Value(VALUE(0x1000))
62366259
v3:Fixnum[1] = Const Value(1)
6237-
v10:BasicObject = SendWithoutBlock v3, :to_s
6238-
v7:String = AnyToString v3, str: v10
6239-
SideExit
6260+
v11:BasicObject = SendWithoutBlock v3, :to_s
6261+
v7:String = AnyToString v3, str: v11
6262+
v9:StringExact = ConcatStrings v2, v7
6263+
Return v9
62406264
"#]]);
62416265
}
62426266

0 commit comments

Comments
 (0)