Skip to content

Commit 6c0a74e

Browse files
authored
ZJIT: Refactor NewRangeFixnum (#14607)
## Context #14409 #14409 (comment) >You may have noticed that this doesn't produce a New range instruction. For that you probably need to use a local variable (makes it harder for the bytecode compiler to reason about, but the JIT sees through it). If you have time, please rewrite these tests. #14409 (comment) >There's some code above to do this for you: see arguments_likely_fixnums and coerce something or other ## Changes ### Refactor tests Modify tests force the usage of `NewRangeFixnum` instruction and make tests names more consistent. ### Refactor optimize ranges Didn't found `arguments_likely_fixnums` to be applicable unless we enable profiling for `NewRange` and change the criteria of when the optimization fires. But there were other ways to clean up the code. Simplify the logic and move it to `type_specialize`. Deleted the standalone `optimize_ranges` function.
1 parent 01de72f commit 6c0a74e

File tree

2 files changed

+45
-68
lines changed

2 files changed

+45
-68
lines changed

test/ruby/test_zjit.rb

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1136,19 +1136,21 @@ def test(n) = n..10
11361136
def test_new_range_fixnum_both_literals_inclusive
11371137
assert_compiles '1..2', %q{
11381138
def test()
1139-
(1..2)
1139+
a = 2
1140+
(1..a)
11401141
end
11411142
test; test
1142-
}, call_threshold: 2
1143+
}, call_threshold: 2, insns: [:newrange]
11431144
end
11441145

11451146
def test_new_range_fixnum_both_literals_exclusive
11461147
assert_compiles '1...2', %q{
11471148
def test()
1148-
(1...2)
1149+
a = 2
1150+
(1...a)
11491151
end
11501152
test; test
1151-
}, call_threshold: 2
1153+
}, call_threshold: 2, insns: [:newrange]
11521154
end
11531155

11541156
def test_new_range_fixnum_low_literal_inclusive
@@ -1157,7 +1159,7 @@ def test(a)
11571159
(1..a)
11581160
end
11591161
test(2); test(3)
1160-
}, call_threshold: 2
1162+
}, call_threshold: 2, insns: [:newrange]
11611163
end
11621164

11631165
def test_new_range_fixnum_low_literal_exclusive
@@ -1166,7 +1168,7 @@ def test(a)
11661168
(1...a)
11671169
end
11681170
test(2); test(3)
1169-
}, call_threshold: 2
1171+
}, call_threshold: 2, insns: [:newrange]
11701172
end
11711173

11721174
def test_new_range_fixnum_high_literal_inclusive
@@ -1175,7 +1177,7 @@ def test(a)
11751177
(a..10)
11761178
end
11771179
test(2); test(3)
1178-
}, call_threshold: 2
1180+
}, call_threshold: 2, insns: [:newrange]
11791181
end
11801182

11811183
def test_new_range_fixnum_high_literal_exclusive
@@ -1184,7 +1186,7 @@ def test(a)
11841186
(a...10)
11851187
end
11861188
test(2); test(3)
1187-
}, call_threshold: 2
1189+
}, call_threshold: 2, insns: [:newrange]
11881190
end
11891191

11901192
def test_if

zjit/src/hir.rs

Lines changed: 35 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1986,6 +1986,20 @@ impl Function {
19861986
self.insn_types[replacement.0] = self.infer_type(replacement);
19871987
self.make_equal_to(insn_id, replacement);
19881988
}
1989+
Insn::NewRange { low, high, flag, state } => {
1990+
let low_is_fix = self.is_a(low, types::Fixnum);
1991+
let high_is_fix = self.is_a(high, types::Fixnum);
1992+
1993+
if low_is_fix || high_is_fix {
1994+
let low_fix = self.coerce_to_fixnum(block, low, state);
1995+
let high_fix = self.coerce_to_fixnum(block, high, state);
1996+
let replacement = self.push_insn(block, Insn::NewRangeFixnum { low: low_fix, high: high_fix, flag, state });
1997+
self.make_equal_to(insn_id, replacement);
1998+
self.insn_types[replacement.0] = self.infer_type(replacement);
1999+
} else {
2000+
self.push_insn_id(block, insn_id);
2001+
};
2002+
}
19892003
_ => { self.push_insn_id(block, insn_id); }
19902004
}
19912005
}
@@ -2194,52 +2208,6 @@ impl Function {
21942208
.unwrap_or(insn_id)
21952209
}
21962210

2197-
fn optimize_ranges(&mut self) {
2198-
for block in self.rpo() {
2199-
let old_insns = std::mem::take(&mut self.blocks[block.0].insns);
2200-
assert!(self.blocks[block.0].insns.is_empty());
2201-
2202-
for insn_id in old_insns {
2203-
match self.find(insn_id) {
2204-
Insn::NewRange { low, high, flag, state } => {
2205-
2206-
// The NewRange rewrite triggers mostly on literals because that is the
2207-
// case we can easily prove Fixnum statically and cheaply guard the other
2208-
// side.
2209-
let low_is_fix = self.is_a(low, types::Fixnum);
2210-
let high_is_fix = self.is_a(high, types::Fixnum);
2211-
2212-
if low_is_fix && high_is_fix {
2213-
// Both statically fixnum => specialize directly
2214-
let repl = self.push_insn(block, Insn::NewRangeFixnum { low, high, flag, state });
2215-
self.make_equal_to(insn_id, repl);
2216-
self.insn_types[repl.0] = self.infer_type(repl);
2217-
} else if low_is_fix {
2218-
// Only left is fixnum => guard right
2219-
let high_fix = self.coerce_to_fixnum(block, high, state);
2220-
let repl = self.push_insn(block, Insn::NewRangeFixnum { low, high: high_fix, flag, state });
2221-
self.make_equal_to(insn_id, repl);
2222-
self.insn_types[repl.0] = self.infer_type(repl);
2223-
} else if high_is_fix {
2224-
// Only right is fixnum => guard left
2225-
let low_fix = self.coerce_to_fixnum(block, low, state);
2226-
let repl = self.push_insn(block, Insn::NewRangeFixnum { low: low_fix, high, flag, state });
2227-
self.make_equal_to(insn_id, repl);
2228-
self.insn_types[repl.0] = self.infer_type(repl);
2229-
} else {
2230-
// Keep generic op
2231-
self.push_insn_id(block, insn_id);
2232-
}
2233-
}
2234-
_ => {
2235-
self.push_insn_id(block, insn_id);
2236-
}
2237-
}
2238-
}
2239-
}
2240-
self.infer_types();
2241-
}
2242-
22432211
/// Use type information left by `infer_types` to fold away operations that can be evaluated at compile-time.
22442212
///
22452213
/// It can fold fixnum math, truthiness tests, and branches with constant conditionals.
@@ -2635,8 +2603,6 @@ impl Function {
26352603
#[cfg(debug_assertions)] self.assert_validates();
26362604
self.optimize_c_calls();
26372605
#[cfg(debug_assertions)] self.assert_validates();
2638-
self.optimize_ranges();
2639-
#[cfg(debug_assertions)] self.assert_validates();
26402606
self.fold_constants();
26412607
#[cfg(debug_assertions)] self.assert_validates();
26422608
self.clean_cfg();
@@ -7253,41 +7219,50 @@ mod opt_tests {
72537219
}
72547220

72557221
#[test]
7256-
fn test_optimize_range_fixnum_inclusive_literals() {
7222+
fn test_optimize_new_range_fixnum_inclusive_literals() {
72577223
eval("
72587224
def test()
7259-
(1..2)
7225+
a = 2
7226+
(1..a)
72607227
end
72617228
test; test
72627229
");
72637230
assert_snapshot!(hir_string("test"), @r"
72647231
fn test@<compiled>:3:
72657232
bb0(v0:BasicObject):
7266-
v4:RangeExact[VALUE(0x1000)] = Const Value(VALUE(0x1000))
7233+
v1:NilClass = Const Value(nil)
7234+
v5:Fixnum[2] = Const Value(2)
7235+
v8:Fixnum[1] = Const Value(1)
7236+
v16:RangeExact = NewRangeFixnum v8 NewRangeInclusive v5
72677237
CheckInterrupts
7268-
Return v4
7238+
Return v16
72697239
");
72707240
}
72717241

7242+
72727243
#[test]
7273-
fn test_optimize_range_fixnum_exclusive_literals() {
7244+
fn test_optimize_new_range_fixnum_exclusive_literals() {
72747245
eval("
72757246
def test()
7276-
(1...2)
7247+
a = 2
7248+
(1...a)
72777249
end
72787250
test; test
72797251
");
72807252
assert_snapshot!(hir_string("test"), @r"
72817253
fn test@<compiled>:3:
72827254
bb0(v0:BasicObject):
7283-
v4:RangeExact[VALUE(0x1000)] = Const Value(VALUE(0x1000))
7255+
v1:NilClass = Const Value(nil)
7256+
v5:Fixnum[2] = Const Value(2)
7257+
v8:Fixnum[1] = Const Value(1)
7258+
v16:RangeExact = NewRangeFixnum v8 NewRangeExclusive v5
72847259
CheckInterrupts
7285-
Return v4
7260+
Return v16
72867261
");
72877262
}
72887263

72897264
#[test]
7290-
fn test_optimize_range_fixnum_inclusive_high_guarded() {
7265+
fn test_optimize_new_range_fixnum_inclusive_high_guarded() {
72917266
eval("
72927267
def test(a)
72937268
(1..a)
@@ -7306,7 +7281,7 @@ mod opt_tests {
73067281
}
73077282

73087283
#[test]
7309-
fn test_optimize_range_fixnum_exclusive_high_guarded() {
7284+
fn test_optimize_new_range_fixnum_exclusive_high_guarded() {
73107285
eval("
73117286
def test(a)
73127287
(1...a)
@@ -7325,7 +7300,7 @@ mod opt_tests {
73257300
}
73267301

73277302
#[test]
7328-
fn test_optimize_range_fixnum_inclusive_low_guarded() {
7303+
fn test_optimize_new_range_fixnum_inclusive_low_guarded() {
73297304
eval("
73307305
def test(a)
73317306
(a..10)
@@ -7344,7 +7319,7 @@ mod opt_tests {
73447319
}
73457320

73467321
#[test]
7347-
fn test_optimize_range_fixnum_exclusive_low_guarded() {
7322+
fn test_optimize_new_range_fixnum_exclusive_low_guarded() {
73487323
eval("
73497324
def test(a)
73507325
(a...10)

0 commit comments

Comments
 (0)