Skip to content

Commit 7699d9b

Browse files
jacob-shopsjacob-shops
committed
Refine get_effects to use information from has_effect
1 parent 4495315 commit 7699d9b

2 files changed

Lines changed: 86 additions & 126 deletions

File tree

‎zjit/src/hir.rs‎

Lines changed: 45 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -1011,122 +1011,54 @@ impl Insn {
10111011
InsnPrinter { inner: self.clone(), ptr_map, iseq }
10121012
}
10131013

1014-
// Unused variables should NOT be allowed. We temporarily allow this to create the skeleton
1015-
// structure for an effects system. Changes that specify refined effects should remove this
1016-
// unused variables attribute.
1017-
#[allow(unused_variables)]
10181014
fn get_effects(&self) -> Effect {
10191015
assert!(self.has_output());
10201016
match &self {
1021-
Insn::Param => unimplemented!("params should not be present in block.insns"),
1022-
Insn::SetGlobal { .. } | Insn::Jump(_) | Insn::EntryPoint { .. }
1023-
| Insn::IfTrue { .. } | Insn::IfFalse { .. } | Insn::Return { .. } | Insn::Throw { .. }
1024-
| Insn::PatchPoint { .. } | Insn::SetIvar { .. } | Insn::SetClassVar { .. } | Insn::ArrayExtend { .. }
1025-
| Insn::ArrayPush { .. } | Insn::SideExit { .. } | Insn::SetLocal { .. } | Insn::IncrCounter(_)
1026-
| Insn::CheckInterrupts { .. } | Insn::GuardBlockParamProxy { .. } | Insn::IncrCounterPtr { .. }
1027-
| Insn::StoreField { .. } | Insn::WriteBarrier { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1028-
Insn::Const { val: Const::Value(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1029-
Insn::Const { val: Const::CBool(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1030-
Insn::Const { val: Const::CInt8(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1031-
Insn::Const { val: Const::CInt16(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1032-
Insn::Const { val: Const::CInt32(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1033-
Insn::Const { val: Const::CInt64(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1034-
Insn::Const { val: Const::CUInt8(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1035-
Insn::Const { val: Const::CUInt16(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1036-
Insn::Const { val: Const::CUInt32(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1037-
Insn::Const { val: Const::CUInt64(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1038-
Insn::Const { val: Const::CPtr(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1039-
Insn::Const { val: Const::CDouble(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1040-
Insn::Test { val } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1041-
Insn::IsNil { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1042-
Insn::IsMethodCfunc { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1043-
Insn::IsBitEqual { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1044-
Insn::IsBitNotEqual { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1045-
Insn::BoxBool { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1046-
Insn::BoxFixnum { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1047-
Insn::UnboxFixnum { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1048-
Insn::StringCopy { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1049-
Insn::StringIntern { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1050-
Insn::StringConcat { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1051-
Insn::StringGetbyte { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1052-
Insn::StringSetbyteFixnum { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1053-
Insn::StringAppend { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1054-
Insn::StringAppendCodepoint { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1055-
Insn::ToRegexp { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1056-
Insn::NewArray { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1057-
Insn::ArrayDup { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1058-
Insn::ArrayArefFixnum { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1059-
Insn::ArrayPop { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1060-
Insn::ArrayLength { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1061-
Insn::HashAref { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1062-
Insn::NewHash { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1063-
Insn::HashDup { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1064-
Insn::NewRange { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1065-
Insn::NewRangeFixnum { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1066-
Insn::ObjectAlloc { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1067-
Insn::ObjectAllocClass { class, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1068-
&Insn::CCallWithFrame { return_type, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1069-
Insn::CCall { return_type, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1070-
&Insn::CCallVariadic { return_type, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1071-
Insn::GuardType { val, guard_type, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1072-
Insn::GuardTypeNot { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1073-
Insn::GuardBitEquals { val, expected, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1074-
Insn::GuardShape { val, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1075-
Insn::GuardNotFrozen { recv, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1076-
Insn::GuardLess { left, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1077-
Insn::GuardGreaterEq { left, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1078-
Insn::FixnumAdd { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1079-
Insn::FixnumSub { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1080-
Insn::FixnumMult { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1081-
Insn::FixnumDiv { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1082-
Insn::FixnumMod { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1083-
Insn::FixnumEq { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1084-
Insn::FixnumNeq { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1085-
Insn::FixnumLt { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1086-
Insn::FixnumLe { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1087-
Insn::FixnumGt { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1088-
Insn::FixnumGe { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1089-
Insn::FixnumAnd { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1090-
Insn::FixnumOr { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1091-
Insn::FixnumXor { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1092-
Insn::FixnumLShift { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1093-
Insn::FixnumRShift { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1094-
Insn::PutSpecialObject { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1095-
Insn::SendWithoutBlock { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1096-
Insn::SendWithoutBlockDirect { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1097-
Insn::Send { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1098-
Insn::SendForward { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1099-
Insn::InvokeSuper { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1100-
Insn::InvokeBlock { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1101-
Insn::InvokeBuiltin { return_type, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1102-
Insn::Defined { pushval, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1103-
Insn::DefinedIvar { pushval, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1104-
Insn::GetConstantPath { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1105-
Insn::IsBlockGiven => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1106-
Insn::FixnumBitCheck { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1107-
Insn::ArrayMax { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1108-
Insn::ArrayInclude { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1109-
Insn::DupArrayInclude { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1110-
Insn::ArrayHash { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1111-
Insn::GetGlobal { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1112-
Insn::GetIvar { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1113-
Insn::LoadPC => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1114-
Insn::LoadEC => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1115-
Insn::LoadSelf => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1116-
&Insn::LoadField { return_type, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1117-
Insn::GetSpecialSymbol { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1118-
Insn::GetSpecialNumber { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1119-
Insn::GetClassVar { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1120-
Insn::ToNewArray { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1121-
Insn::ToArray { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1122-
Insn::ObjToString { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1123-
Insn::AnyToString { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1124-
Insn::GetLocal { rest_param: true, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1125-
Insn::GetLocal { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1126-
// The type of Snapshot doesn't really matter; it's never materialized. It's used only
1127-
// as a reference for FrameState, which we use to generate side-exit code.
1128-
Insn::Snapshot { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1129-
Insn::IsA { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1017+
Insn::Const { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1018+
Insn::Param => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1019+
Insn::StringCopy { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1020+
Insn::NewArray { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1021+
// NewHash's operands may be hashed and compared for equalityEffect::from_bits(effect_sets::Any, effect_sets::Allocator), which could have
1022+
// side-effects.
1023+
// fix newhash which seems to be different than the rest
1024+
// Insn::NewHash { elementsEffect::from_bits(effect_sets::Any, effect_sets::Allocator), .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1025+
Insn::ArrayDup { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1026+
Insn::HashDup { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1027+
Insn::Test { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1028+
Insn::Snapshot { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1029+
Insn::FixnumAdd { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1030+
Insn::FixnumSub { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1031+
Insn::FixnumMult { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1032+
Insn::FixnumEq { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1033+
Insn::FixnumNeq { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1034+
Insn::FixnumLt { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1035+
Insn::FixnumLe { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1036+
Insn::FixnumGt { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1037+
Insn::FixnumGe { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1038+
Insn::FixnumAnd { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1039+
Insn::FixnumOr { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1040+
Insn::FixnumXor { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1041+
Insn::FixnumLShift { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1042+
Insn::FixnumRShift { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1043+
Insn::GetLocal { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1044+
Insn::IsNil { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1045+
Insn::LoadPC => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1046+
Insn::LoadEC => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1047+
Insn::LoadSelf => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1048+
Insn::LoadField { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1049+
// Special case, let's fix this
1050+
// Insn::CCall { elidable, .. } => !elidable Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1051+
// Special case, let's fix this one too
1052+
// Insn::CCallWithFrame { elidable, .. } => !elidable Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1053+
Insn::ObjectAllocClass { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1054+
Insn::NewRangeFixnum { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1055+
Insn::StringGetbyte { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1056+
Insn::IsBlockGiven => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1057+
Insn::BoxFixnum { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1058+
Insn::BoxBool { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1059+
Insn::IsBitEqual { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1060+
Insn::IsA { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1061+
_ => Effect::from_bits(effect_sets::Any, effect_sets::Any),
11301062
}
11311063
}
11321064

‎zjit/src/hir_effect/mod.rs‎

Lines changed: 41 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ type EffectBits = u8;
99

1010
// NOTE: Effect very intentionally does not support Eq or PartialEq; we almost never want to check
1111
// bit equality of types in the compiler but instead check subtyping, intersection, union, etc.
12-
#[derive(Copy, Clone, Debug)]
1312
/// The main work horse of effect inference and specialization. The main interfaces
1413
/// will look like:
1514
///
@@ -24,6 +23,9 @@ type EffectBits = u8;
2423
/// This enables more complex analyses compared to prior ZJIT implementations such as "has_effect",
2524
/// a function that returns a boolean value. Such functions impose an implicit single bit effect
2625
/// system. This explicit design with a lattice allows us many bits for effects.
26+
// TODO(Jacob): Fix orphan rule and implement partialord
27+
// #[derive(PartialEq, PartialOrd)]
28+
#[derive(Clone, Copy, Debug)]
2729
pub struct EffectSet {
2830
bits: EffectBits
2931
}
@@ -89,32 +91,32 @@ impl std::fmt::Display for EffectSet {
8991
// TODO(Jacob): Modify union and effect to work on an arbitrary number of args
9092
// TODO(Jacob): These `from_bits` functions used to be `const fn` not `pub fn`. Have I done something bad by making them public?
9193
impl EffectSet {
92-
const fn from_bits(bits: EffectBits) -> EffectSet {
93-
EffectSet { bits }
94+
const fn from_bits(bits: EffectBits) -> Self {
95+
Self { bits }
9496
}
9597

96-
pub fn union(&self, other: EffectSet) -> EffectSet {
97-
EffectSet::from_bits(self.bits | other.bits)
98+
pub fn union(&self, other: Self) -> Self {
99+
Self::from_bits(self.bits | other.bits)
98100
}
99101

100-
pub fn intersect(&self, other: EffectSet) -> EffectSet {
101-
EffectSet::from_bits(self.bits & other.bits)
102+
pub fn intersect(&self, other: Self) -> Self {
103+
Self::from_bits(self.bits & other.bits)
102104
}
103105

104-
pub fn exclude(&self, other: EffectSet) -> EffectSet {
105-
EffectSet::from_bits(self.bits - (self.bits & other.bits))
106+
pub fn exclude(&self, other: Self) -> Self {
107+
Self::from_bits(self.bits - (self.bits & other.bits))
106108
}
107109

108110
/// Check bit equality of two `Effect`s. Do not use! You are probably looking for [`Effect::includes`].
109-
pub fn bit_equal(&self, other: EffectSet) -> bool {
111+
pub fn bit_equal(&self, other: Self) -> bool {
110112
self.bits == other.bits
111113
}
112114

113-
pub fn includes(&self, other: EffectSet) -> bool {
114-
self.bit_equal(EffectSet::union(self, other))
115+
pub fn includes(&self, other: Self) -> bool {
116+
self.bit_equal(Self::union(self, other))
115117
}
116118

117-
pub fn overlaps(&self, other: EffectSet) -> bool {
119+
pub fn overlaps(&self, other: Self) -> bool {
118120
!self.intersect(other).bit_equal(effect_sets::Empty)
119121
}
120122

@@ -123,6 +125,32 @@ impl EffectSet {
123125
}
124126
}
125127

128+
// impl PartialEq for EffectSet {
129+
// fn eq(&self, other: Self) -> bool {
130+
// self.bit_equal(other)
131+
// }
132+
// }
133+
134+
// TODO(Jacob): Clean up this logic. It can be simplified with one `mut`
135+
// impl PartialOrd for EffectSet {
136+
// fn partial_cmp(&self, other: Self) -> Option<std::cmp::Ordering> {
137+
// if self.bit_equal(other) {
138+
// return Some(std::cmp::Ordering::Equal)
139+
// }
140+
// else if self.includes(other) {
141+
// return Some(std::cmp::Ordering::Greater)
142+
// }
143+
// else if other.includes(*self) {
144+
// return Some(std::cmp::Ordering::Less)
145+
// }
146+
// // If one EffectSet is not included in another, they cannot be compared
147+
// // This case corresponds to different branches in the lattice
148+
// else {
149+
// return None
150+
// }
151+
// }
152+
// }
153+
126154
impl Effect {
127155
pub fn from_bits(read: EffectSet, write: EffectSet) -> Effect {
128156
Effect { read, write }

0 commit comments

Comments
 (0)