Skip to content

Commit d8177d6

Browse files
authored
ZJIT: Move string encoding flags loading to HIR (ruby#18894)
Closes Shopify#1051 The LIR generated without stats is the same, but now the `LoadField` can be deduped. Same as ruby#18251 but without moving the `xor` to the HIR. Generate HIR is now: ``` v31:CUInt64 = LoadField v29, :RBASIC_FLAGS@0x1040 v32:CUInt64 = LoadField v30, :RBASIC_FLAGS@0x1040 v33:StringExact = StringAppend v29, v30, recv_flags: v31, other_flags: v32 ``` Compared with: ``` v31:StringExact = StringAppend v29, v30 ``` ### Benchmark ``` before: ruby 4.1.0dev (2026-09-17T02:33:06Z :detached: e6ec155) +PRISM [arm64-darwin25] after: ruby 4.1.0dev (2026-09-17T03:27:34Z :detached: fc9a90e) +PRISM [arm64-darwin25] ---------- ------------- ------------- ------------- ------------ bench before (ms) after (ms) after 1st itr before/after lobsters 636.7 ± 5.0% 631.6 ± 2.6% 0.999 1.008 railsbench 1175.6 ± 1.3% 1171.6 ± 1.3% 0.978 1.003 str_concat 41.7 ± 2.4% 42.3 ± 3.4% 1.006 0.984 ---------- ------------- ------------- ------------- ------------ Legend: - after 1st itr: ratio of before/after time for the first benchmarking iteration. - before/after: ratio of before/after time. Higher is better for after. Above 1 represents a speedup. ```
1 parent 15dd95c commit d8177d6

5 files changed

Lines changed: 101 additions & 27 deletions

File tree

‎zjit/src/codegen.rs‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -675,7 +675,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
675675
&Insn::StringGetbyte { string, index } => gen_string_getbyte(asm, opnd!(string), opnd!(index)),
676676
Insn::StringByteslice { string, beg, len, state } => gen_string_byteslice(asm, opnd!(string), opnd!(beg), opnd!(len), &function.frame_state(*state)),
677677
Insn::StringSetbyteFixnum { string, index, value } => gen_string_setbyte_fixnum(asm, opnd!(string), opnd!(index), opnd!(value)),
678-
Insn::StringAppend { recv, other, state } => gen_string_append(jit, asm, function, opnd!(recv), opnd!(other), &function.frame_state(*state)),
678+
Insn::StringAppend { recv, other, recv_flags, other_flags, state } => gen_string_append(jit, asm, function, opnd!(recv), opnd!(other), opnd!(recv_flags), opnd!(other_flags), &function.frame_state(*state)),
679679
Insn::StringAppendCodepoint { recv, other, state } => gen_string_append_codepoint(jit, asm, function, opnd!(recv), opnd!(other), &function.frame_state(*state)),
680680
Insn::StringEqual { left, right } => gen_string_equal(asm, opnd!(left), opnd!(right)),
681681
Insn::StringIntern { val, state } => gen_intern(jit, asm, function, opnd!(val), &function.frame_state(*state)),
@@ -4246,22 +4246,16 @@ fn gen_string_setbyte_fixnum(asm: &mut Assembler, string: Opnd, index: Opnd, val
42464246
asm_ccall!(asm, rb_str_setbyte, string, index, value)
42474247
}
42484248

4249-
fn gen_string_append(jit: &mut JITState, asm: &mut Assembler, function: &Function, string: Opnd, val: Opnd, state: &FrameState) -> Opnd {
4249+
fn gen_string_append(jit: &mut JITState, asm: &mut Assembler, function: &Function, string: Opnd, val: Opnd, recv_flags: Opnd, other_flags: Opnd, state: &FrameState) -> Opnd {
42504250
gen_prepare_non_leaf_call(jit, asm, function, state);
42514251

42524252
// Test if string encodings differ. If different, use rb_str_buf_append. If the same,
42534253
// use rb_jit_str_simple_append, which calls rb_str_cat.
42544254
asm_comment!(asm, "<< on strings");
42554255

4256-
// Take receiver's object flags XOR arg's flags. If any
4257-
// string-encoding flags are different between the two,
4258-
// the encodings don't match.
4259-
let string_reg = asm.load_mem(string);
4260-
let val_reg = asm.load_mem(val);
4261-
let flags_xor = asm.xor(
4262-
Opnd::mem(VALUE_BITS, string_reg, RUBY_OFFSET_RBASIC_FLAGS),
4263-
Opnd::mem(VALUE_BITS, val_reg, RUBY_OFFSET_RBASIC_FLAGS)
4264-
);
4256+
// XOR the object flags. If any string-encoding flags differ between the two,
4257+
// the encodings do not match.
4258+
let flags_xor = asm.xor(recv_flags, other_flags);
42654259
asm.test(flags_xor, Opnd::UImm(RUBY_ENCODING_MASK as u64));
42664260

42674261
let hir_block_id = asm.current_block().hir_block_id;

‎zjit/src/codegen_tests.rs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4568,6 +4568,26 @@ fn test_string_append_encoding_mismatch() {
45684568
"#), @r#"["éé", "UTF-8", true]"#);
45694569
}
45704570

4571+
#[test]
4572+
fn test_string_append_encoding_mutation_between_appends() {
4573+
eval(r#"
4574+
def test(string, first, second)
4575+
string << first
4576+
string << second
4577+
end
4578+
"#);
4579+
assert_contains_opcode("test", YARVINSN_opt_ltlt);
4580+
assert_snapshot!(assert_compiles(r#"
4581+
string = String.new(encoding: Encoding::BINARY)
4582+
begin
4583+
test(string, "é", "\xFF".b)
4584+
:no_error
4585+
rescue Encoding::CompatibilityError
4586+
[string.bytes, string.encoding.name, string.valid_encoding?]
4587+
end
4588+
"#), @"[[195, 169], \"UTF-8\", true]");
4589+
}
4590+
45714591
#[test]
45724592
fn test_string_append_incompatible_encoding() {
45734593
eval(r#"

‎zjit/src/cruby_methods.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -592,7 +592,9 @@ fn inline_string_append(fun: &mut hir::Function, block: hir::BlockId, recv: hir:
592592
if fun.likely_a(recv, types::StringExact, state) && fun.likely_a(other, types::String, state) {
593593
let recv = fun.coerce_to(block, recv, types::StringExact, state);
594594
let other = fun.coerce_to(block, other, types::String, state);
595-
let _ = fun.push_insn(block, hir::Insn::StringAppend { recv, other, state });
595+
let recv_flags = fun.load_rbasic_flags(block, recv);
596+
let other_flags = fun.load_rbasic_flags(block, other);
597+
let _ = fun.push_insn(block, hir::Insn::StringAppend { recv, other, recv_flags, other_flags, state });
596598
return Some(recv);
597599
}
598600
if fun.likely_a(recv, types::StringExact, state) && fun.likely_a(other, types::Fixnum, state) {

‎zjit/src/hir.rs‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,7 +1017,8 @@ pub enum Insn {
10171017
/// Call rb_str_byte_substr with known-Fixnum beg/len
10181018
StringByteslice { string: InsnId, beg: InsnId, len: InsnId, state: InsnId },
10191019
StringSetbyteFixnum { string: InsnId, index: InsnId, value: InsnId },
1020-
StringAppend { recv: InsnId, other: InsnId, state: InsnId },
1020+
/// Append `other` to `recv`. HIR loads both flags for load reuse. Codegen XORs the flags.
1021+
StringAppend { recv: InsnId, other: InsnId, recv_flags: InsnId, other_flags: InsnId, state: InsnId },
10211022
StringAppendCodepoint { recv: InsnId, other: InsnId, state: InsnId },
10221023
StringEqual { left: InsnId, right: InsnId },
10231024

@@ -1452,8 +1453,14 @@ macro_rules! for_each_operand_impl {
14521453
$visit_one!(*index);
14531454
$visit_one!(*value);
14541455
}
1455-
Insn::StringAppend { recv, other, state }
1456-
| Insn::StringAppendCodepoint { recv, other, state } => {
1456+
Insn::StringAppend { recv, other, recv_flags, other_flags, state } => {
1457+
$visit_one!(*recv);
1458+
$visit_one!(*other);
1459+
$visit_one!(*recv_flags);
1460+
$visit_one!(*other_flags);
1461+
$visit_one!(*state);
1462+
}
1463+
Insn::StringAppendCodepoint { recv, other, state } => {
14571464
$visit_one!(*recv);
14581465
$visit_one!(*other);
14591466
$visit_one!(*state);
@@ -2180,8 +2187,8 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> {
21802187
Insn::StringSetbyteFixnum { string, index, value, .. } => {
21812188
write!(f, "StringSetbyteFixnum {string}, {index}, {value}")
21822189
}
2183-
Insn::StringAppend { recv, other, .. } => {
2184-
write!(f, "StringAppend {recv}, {other}")
2190+
Insn::StringAppend { recv, other, recv_flags, other_flags, .. } => {
2191+
write!(f, "StringAppend {recv}, {other}, recv_flags: {recv_flags}, other_flags: {other_flags}")
21852192
}
21862193
Insn::StringAppendCodepoint { recv, other, .. } => {
21872194
write!(f, "StringAppendCodepoint {recv}, {other}")
@@ -8015,9 +8022,11 @@ impl Function {
80158022
// Instructions with String operands
80168023
Insn::StringCopy { val, .. } => self.assert_subtype(insn_id, val, types::StringExact),
80178024
Insn::StringIntern { val, .. } => self.assert_subtype(insn_id, val, types::StringExact),
8018-
Insn::StringAppend { recv, other, .. } => {
8025+
Insn::StringAppend { recv, other, recv_flags, other_flags, .. } => {
80198026
self.assert_subtype(insn_id, recv, types::StringExact)?;
8020-
self.assert_subtype(insn_id, other, types::String)
8027+
self.assert_subtype(insn_id, other, types::String)?;
8028+
self.assert_subtype(insn_id, recv_flags, types::CUInt64)?;
8029+
self.assert_subtype(insn_id, other_flags, types::CUInt64)
80218030
}
80228031
Insn::StringAppendCodepoint { recv, other, .. } => {
80238032
self.assert_subtype(insn_id, recv, types::StringExact)?;

‎zjit/src/hir/opt_tests.rs‎

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14540,12 +14540,57 @@ mod hir_opt_tests {
1454014540
PatchPoint MethodRedefined(String@0x1008, <<@0x1010, cme:0x1018)
1454114541
v29:StringExact = GuardType v12, StringExact recompile
1454214542
v30:String = GuardType v13, String
14543-
v31:StringExact = StringAppend v29, v30
14543+
v31:CUInt64 = LoadField v29, :RBASIC_FLAGS@0x1040
14544+
v32:CUInt64 = LoadField v30, :RBASIC_FLAGS@0x1040
14545+
v33:StringExact = StringAppend v29, v30, recv_flags: v31, other_flags: v32
1454414546
CheckInterrupts
1454514547
Return v29
1454614548
");
1454714549
}
1454814550

14551+
#[test]
14552+
fn test_optimize_string_append_reuses_flags_load() {
14553+
eval(r#"
14554+
def test(s, t)
14555+
s.ascii_only?
14556+
s << t
14557+
end
14558+
test("iron", "fish")
14559+
"#);
14560+
assert_snapshot!(hir_string("test"), @"
14561+
fn test@<compiled>:3:
14562+
bb1():
14563+
EntryPoint interpreter
14564+
v1:BasicObject = LoadSelf
14565+
v2:CPtr = LoadSP
14566+
v3:BasicObject = LoadField v2, :s@0x1000
14567+
v4:BasicObject = LoadField v2, :t@0x1001
14568+
Jump bb3(v1, v3, v4)
14569+
bb2():
14570+
EntryPoint JIT(0)
14571+
v7:BasicObject = LoadArg :self@0
14572+
v8:BasicObject = LoadArg :s@1
14573+
v9:BasicObject = LoadArg :t@2
14574+
Jump bb3(v7, v8, v9)
14575+
bb3(v11:BasicObject, v12:BasicObject, v13:BasicObject):
14576+
PatchPoint NoSingletonClass(String@0x1008)
14577+
PatchPoint MethodRedefined(String@0x1008, ascii_only?@0x1010, cme:0x1018)
14578+
v35:StringExact = GuardType v12, StringExact recompile
14579+
v36:CUInt64 = LoadField v35, :RBASIC_FLAGS@0x1040
14580+
v37:CUInt64[3145728] = Const CUInt64(3145728)
14581+
v38:CInt64 = IntAnd v36, v37
14582+
v39:CInt64[1048576] = Const CInt64(1048576)
14583+
v40:CInt64 = GuardGreaterEq v38, v39
14584+
PatchPoint NoEPEscape(test)
14585+
PatchPoint MethodRedefined(String@0x1008, <<@0x1041, cme:0x1048)
14586+
v48:String = GuardType v13, String
14587+
v50:CUInt64 = LoadField v48, :RBASIC_FLAGS@0x1040
14588+
v51:StringExact = StringAppend v35, v48, recv_flags: v36, other_flags: v50
14589+
CheckInterrupts
14590+
Return v35
14591+
");
14592+
}
14593+
1454914594
#[test]
1455014595
fn test_optimize_string_append_codepoint() {
1455114596
eval(r#"
@@ -14606,7 +14651,9 @@ mod hir_opt_tests {
1460614651
PatchPoint MethodRedefined(String@0x1008, <<@0x1010, cme:0x1018)
1460714652
v29:StringExact = GuardType v12, StringExact recompile
1460814653
v30:String = GuardType v13, String
14609-
v31:StringExact = StringAppend v29, v30
14654+
v31:CUInt64 = LoadField v29, :RBASIC_FLAGS@0x1040
14655+
v32:CUInt64 = LoadField v30, :RBASIC_FLAGS@0x1040
14656+
v33:StringExact = StringAppend v29, v30, recv_flags: v31, other_flags: v32
1461014657
CheckInterrupts
1461114658
Return v29
1461214659
");
@@ -17059,8 +17106,8 @@ mod hir_opt_tests {
1705917106
v6:BasicObject = LoadArg :self@0
1706017107
Jump bb3(v6)
1706117108
bb3(v10:BasicObject):
17062-
v57:NilClass = Const Value(nil)
17063-
v56:NilClass = Const Value(nil)
17109+
v59:NilClass = Const Value(nil)
17110+
v58:NilClass = Const Value(nil)
1706417111
v16:StringExact[VALUE(0x1000)] = Const Value(VALUE(0x1000))
1706517112
v17:StringExact = StringCopy v16
1706617113
v21:StringExact[VALUE(0x1000)] = Const Value(VALUE(0x1000))
@@ -17069,13 +17116,15 @@ mod hir_opt_tests {
1706917116
v28:StringExact = StringCopy v27
1707017117
PatchPoint NoSingletonClass(String@0x1008)
1707117118
PatchPoint MethodRedefined(String@0x1008, <<@0x1010, cme:0x1018)
17072-
v50:StringExact = StringAppend v17, v28
17119+
v50:CUInt64 = LoadField v17, :RBASIC_FLAGS@0x1040
17120+
v51:CUInt64 = LoadField v28, :RBASIC_FLAGS@0x1040
17121+
v52:StringExact = StringAppend v17, v28, recv_flags: v50, other_flags: v51
1707317122
PatchPoint NoEPEscape(test)
1707417123
PatchPoint NoSingletonClass(String@0x1008)
17075-
PatchPoint MethodRedefined(String@0x1008, ==@0x1040, cme:0x1048)
17076-
v55:BoolExact = StringEqual v17, v22
17124+
PatchPoint MethodRedefined(String@0x1008, ==@0x1041, cme:0x1048)
17125+
v57:BoolExact = StringEqual v17, v22
1707717126
CheckInterrupts
17078-
Return v55
17127+
Return v57
1707917128
");
1708017129
}
1708117130

0 commit comments

Comments
 (0)