Skip to content

Commit 6b197de

Browse files
committed
ZJIT: Mark Insn::NewRange as having side effects
1 parent e639aaa commit 6b197de

File tree

2 files changed

+27
-1
lines changed

2 files changed

+27
-1
lines changed

range.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ static VALUE r_cover_p(VALUE, VALUE, VALUE, VALUE);
4747
static void
4848
range_init(VALUE range, VALUE beg, VALUE end, VALUE exclude_end)
4949
{
50+
// Changing this condition has implications for JITs. If you do, please let maintainers know.
5051
if ((!FIXNUM_P(beg) || !FIXNUM_P(end)) && !NIL_P(beg) && !NIL_P(end)) {
5152
VALUE v;
5253

zjit/src/hir.rs

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -638,7 +638,6 @@ impl Insn {
638638
// NewHash's operands may be hashed and compared for equality, which could have
639639
// side-effects.
640640
Insn::NewHash { elements, .. } => elements.len() > 0,
641-
Insn::NewRange { .. } => false,
642641
Insn::ArrayDup { .. } => false,
643642
Insn::HashDup { .. } => false,
644643
Insn::Test { .. } => false,
@@ -660,6 +659,9 @@ impl Insn {
660659
Insn::GetLocal { .. } => false,
661660
Insn::IsNil { .. } => false,
662661
Insn::CCall { elidable, .. } => !elidable,
662+
// TODO: NewRange is effects free if we can prove the two ends to be Fixnum,
663+
// but we don't have type information here in `impl Insn`. See rb_range_new().
664+
Insn::NewRange { .. } => true,
663665
_ => true,
664666
}
665667
}
@@ -6198,6 +6200,29 @@ mod opt_tests {
61986200
"#]]);
61996201
}
62006202

6203+
#[test]
6204+
fn test_do_not_eliminate_new_range_non_fixnum() {
6205+
eval("
6206+
def test()
6207+
_ = (-'a'..'b')
6208+
0
6209+
end
6210+
test; test
6211+
");
6212+
assert_optimized_method_hir("test", expect![[r#"
6213+
fn test@<compiled>:3:
6214+
bb0(v0:BasicObject):
6215+
v1:NilClass = Const Value(nil)
6216+
v4:StringExact[VALUE(0x1000)] = Const Value(VALUE(0x1000))
6217+
PatchPoint BOPRedefined(STRING_REDEFINED_OP_FLAG, BOP_UMINUS)
6218+
v6:StringExact[VALUE(0x1008)] = Const Value(VALUE(0x1008))
6219+
v8:StringExact = StringCopy v6
6220+
v10:RangeExact = NewRange v4 NewRangeInclusive v8
6221+
v11:Fixnum[0] = Const Value(0)
6222+
Return v11
6223+
"#]]);
6224+
}
6225+
62016226
#[test]
62026227
fn test_eliminate_new_array_with_elements() {
62036228
eval("

0 commit comments

Comments
 (0)