Skip to content

Commit ca78a6c

Browse files
tekknolagitekknolagi
committed
ZJIT: Clean up partial SSI
After Kokubun requested named unions, I realized we don't actually need a `Type::subtract` function. They were only used for the ad-hoc unions. Also, add a test that is illustrative of what we can get from this partial SSI.
1 parent 436ec3a commit ca78a6c

File tree

4 files changed

+117
-65
lines changed

4 files changed

+117
-65
lines changed

‎zjit/src/hir.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6293,15 +6293,15 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
62936293
let test_id = fun.push_insn(block, Insn::Test { val });
62946294
let target_idx = insn_idx_at_offset(insn_idx, offset);
62956295
let target = insn_idx_to_block[&target_idx];
6296-
let nil_false_type = types::NilClass.union(types::FalseClass);
6296+
let nil_false_type = types::Falsy;
62976297
let nil_false = fun.push_insn(block, Insn::RefineType { val, new_type: nil_false_type });
62986298
let mut iffalse_state = state.clone();
62996299
iffalse_state.replace(val, nil_false);
63006300
let _branch_id = fun.push_insn(block, Insn::IfFalse {
63016301
val: test_id,
63026302
target: BranchEdge { target, args: iffalse_state.as_args(self_param) }
63036303
});
6304-
let not_nil_false_type = types::BasicObject.subtract(types::NilClass).subtract(types::FalseClass);
6304+
let not_nil_false_type = types::Truthy;
63056305
let not_nil_false = fun.push_insn(block, Insn::RefineType { val, new_type: not_nil_false_type });
63066306
state.replace(val, not_nil_false);
63076307
queue.push_back((state.clone(), target, target_idx, local_inval));
@@ -6313,15 +6313,15 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
63136313
let test_id = fun.push_insn(block, Insn::Test { val });
63146314
let target_idx = insn_idx_at_offset(insn_idx, offset);
63156315
let target = insn_idx_to_block[&target_idx];
6316-
let not_nil_false_type = types::BasicObject.subtract(types::NilClass).subtract(types::FalseClass);
6316+
let not_nil_false_type = types::Truthy;
63176317
let not_nil_false = fun.push_insn(block, Insn::RefineType { val, new_type: not_nil_false_type });
63186318
let mut iftrue_state = state.clone();
63196319
iftrue_state.replace(val, not_nil_false);
63206320
let _branch_id = fun.push_insn(block, Insn::IfTrue {
63216321
val: test_id,
63226322
target: BranchEdge { target, args: iftrue_state.as_args(self_param) }
63236323
});
6324-
let nil_false_type = types::NilClass.union(types::FalseClass);
6324+
let nil_false_type = types::Falsy;
63256325
let nil_false = fun.push_insn(block, Insn::RefineType { val, new_type: nil_false_type });
63266326
state.replace(val, nil_false);
63276327
queue.push_back((state.clone(), target, target_idx, local_inval));
@@ -6340,7 +6340,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
63406340
val: test_id,
63416341
target: BranchEdge { target, args: iftrue_state.as_args(self_param) }
63426342
});
6343-
let new_type = types::BasicObject.subtract(types::NilClass);
6343+
let new_type = types::NotNil;
63446344
let not_nil = fun.push_insn(block, Insn::RefineType { val, new_type });
63456345
state.replace(val, not_nil);
63466346
queue.push_back((state.clone(), target, target_idx, local_inval));

‎zjit/src/hir/opt_tests.rs‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11319,4 +11319,53 @@ mod hir_opt_tests {
1131911319
Return v31
1132011320
");
1132111321
}
11322+
11323+
#[test]
11324+
fn test_infer_truthiness_from_branch() {
11325+
eval("
11326+
def test(x)
11327+
if x
11328+
if x
11329+
if x
11330+
3
11331+
else
11332+
4
11333+
end
11334+
else
11335+
5
11336+
end
11337+
else
11338+
6
11339+
end
11340+
end
11341+
");
11342+
assert_snapshot!(hir_string("test"), @r"
11343+
fn test@<compiled>:3:
11344+
bb0():
11345+
EntryPoint interpreter
11346+
v1:BasicObject = LoadSelf
11347+
v2:BasicObject = GetLocal :x, l0, SP@4
11348+
Jump bb2(v1, v2)
11349+
bb1(v5:BasicObject, v6:BasicObject):
11350+
EntryPoint JIT(0)
11351+
Jump bb2(v5, v6)
11352+
bb2(v8:BasicObject, v9:BasicObject):
11353+
CheckInterrupts
11354+
v15:CBool = Test v9
11355+
v16:Falsy = RefineType v9, Falsy
11356+
IfFalse v15, bb5(v8, v16)
11357+
v18:Truthy = RefineType v9, Truthy
11358+
CheckInterrupts
11359+
v26:Truthy = RefineType v18, Truthy
11360+
CheckInterrupts
11361+
v34:Truthy = RefineType v26, Truthy
11362+
v37:Fixnum[3] = Const Value(3)
11363+
CheckInterrupts
11364+
Return v37
11365+
bb5(v42:BasicObject, v43:Falsy):
11366+
v47:Fixnum[6] = Const Value(6)
11367+
CheckInterrupts
11368+
Return v47
11369+
");
11370+
}
1132211371
}

‎zjit/src/hir/tests.rs‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3222,6 +3222,69 @@ pub mod hir_build_tests {
32223222
");
32233223
}
32243224

3225+
#[test]
3226+
fn test_infer_truthiness_from_branch() {
3227+
eval("
3228+
def test(x)
3229+
if x
3230+
if x
3231+
if x
3232+
3
3233+
else
3234+
4
3235+
end
3236+
else
3237+
5
3238+
end
3239+
else
3240+
6
3241+
end
3242+
end
3243+
");
3244+
assert_snapshot!(hir_string("test"), @r"
3245+
fn test@<compiled>:3:
3246+
bb0():
3247+
EntryPoint interpreter
3248+
v1:BasicObject = LoadSelf
3249+
v2:BasicObject = GetLocal :x, l0, SP@4
3250+
Jump bb2(v1, v2)
3251+
bb1(v5:BasicObject, v6:BasicObject):
3252+
EntryPoint JIT(0)
3253+
Jump bb2(v5, v6)
3254+
bb2(v8:BasicObject, v9:BasicObject):
3255+
CheckInterrupts
3256+
v15:CBool = Test v9
3257+
v16:Falsy = RefineType v9, Falsy
3258+
IfFalse v15, bb5(v8, v16)
3259+
v18:Truthy = RefineType v9, Truthy
3260+
CheckInterrupts
3261+
v23:CBool[true] = Test v18
3262+
v24 = RefineType v18, Falsy
3263+
IfFalse v23, bb4(v8, v24)
3264+
v26:Truthy = RefineType v18, Truthy
3265+
CheckInterrupts
3266+
v31:CBool[true] = Test v26
3267+
v32 = RefineType v26, Falsy
3268+
IfFalse v31, bb3(v8, v32)
3269+
v34:Truthy = RefineType v26, Truthy
3270+
v37:Fixnum[3] = Const Value(3)
3271+
CheckInterrupts
3272+
Return v37
3273+
bb5(v42:BasicObject, v43:Falsy):
3274+
v47:Fixnum[6] = Const Value(6)
3275+
CheckInterrupts
3276+
Return v47
3277+
bb4(v52, v53):
3278+
v57 = Const Value(5)
3279+
CheckInterrupts
3280+
Return v57
3281+
bb3(v62, v63):
3282+
v67 = Const Value(4)
3283+
CheckInterrupts
3284+
Return v67
3285+
");
3286+
}
3287+
32253288
#[test]
32263289
fn test_invokebuiltin_delegate_annotated() {
32273290
assert_contains_opcode("Float", YARVINSN_opt_invokebuiltin_delegate_leave);

‎zjit/src/hir_type/mod.rs‎

Lines changed: 0 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -453,25 +453,6 @@ impl Type {
453453
types::Empty
454454
}
455455

456-
/// Subtract `other` from `self`, preserving specialization if possible.
457-
pub fn subtract(&self, other: Type) -> Type {
458-
// If self is a subtype of other, the result is empty (no negative types).
459-
if self.is_subtype(other) { return types::Empty; }
460-
// Self is not a subtype of other. That means either:
461-
// * Their type bits do not overlap at all (eg Int vs String)
462-
// * Their type bits overlap but self's specialization is not a subtype of other's (eg
463-
// Fixnum[5] vs Fixnum[4])
464-
// Check for the latter case, returning self unchanged if so.
465-
if !self.spec_is_subtype_of(other) {
466-
return *self;
467-
}
468-
// Now self is either a supertype of other (eg Object vs String or Fixnum vs Fixnum[5]) or
469-
// their type bits do not overlap at all (eg Int vs String).
470-
// Just subtract the bits and keep self's specialization.
471-
let bits = self.bits & !other.bits;
472-
Type { bits, spec: self.spec }
473-
}
474-
475456
pub fn could_be(&self, other: Type) -> bool {
476457
!self.intersection(other).bit_equal(types::Empty)
477458
}
@@ -1079,45 +1060,4 @@ mod tests {
10791060
assert!(!types::CBool.has_value(Const::CBool(true)));
10801061
assert!(!types::CShape.has_value(Const::CShape(crate::cruby::ShapeId(0x1234))));
10811062
}
1082-
1083-
#[test]
1084-
fn test_subtract_with_superset_returns_empty() {
1085-
let left = types::NilClass;
1086-
let right = types::BasicObject;
1087-
let result = left.subtract(right);
1088-
assert_bit_equal(result, types::Empty);
1089-
}
1090-
1091-
#[test]
1092-
fn test_subtract_with_subset_removes_bits() {
1093-
let left = types::BasicObject;
1094-
let right = types::NilClass;
1095-
let result = left.subtract(right);
1096-
assert_subtype(result, types::BasicObject);
1097-
assert_not_subtype(types::NilClass, result);
1098-
}
1099-
1100-
#[test]
1101-
fn test_subtract_with_no_overlap_returns_self() {
1102-
let left = types::Fixnum;
1103-
let right = types::StringExact;
1104-
let result = left.subtract(right);
1105-
assert_bit_equal(result, left);
1106-
}
1107-
1108-
#[test]
1109-
fn test_subtract_with_no_specialization_overlap_returns_self() {
1110-
let left = Type::fixnum(4);
1111-
let right = Type::fixnum(5);
1112-
let result = left.subtract(right);
1113-
assert_bit_equal(result, left);
1114-
}
1115-
1116-
#[test]
1117-
fn test_subtract_with_specialization_subset_removes_specialization() {
1118-
let left = types::Fixnum;
1119-
let right = Type::fixnum(42);
1120-
let result = left.subtract(right);
1121-
assert_bit_equal(result, types::Fixnum);
1122-
}
11231063
}

0 commit comments

Comments
 (0)