Skip to content

Commit 93bb30e

Browse files
jacob-shopsjacob-shops
committed
Refine get_effects to use information from has_effect
1 parent 9f2a14b commit 93bb30e

File tree

2 files changed

+86
-126
lines changed

2 files changed

+86
-126
lines changed

‎zjit/src/hir.rs‎

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

972-
// Unused variables should NOT be allowed. We temporarily allow this to create the skeleton
973-
// structure for an effects system. Changes that specify refined effects should remove this
974-
// unused variables attribute.
975-
#[allow(unused_variables)]
976972
fn get_effects(&self) -> Effect {
977973
assert!(self.has_output());
978974
match &self {
979-
Insn::Param => unimplemented!("params should not be present in block.insns"),
980-
Insn::SetGlobal { .. } | Insn::Jump(_) | Insn::EntryPoint { .. }
981-
| Insn::IfTrue { .. } | Insn::IfFalse { .. } | Insn::Return { .. } | Insn::Throw { .. }
982-
| Insn::PatchPoint { .. } | Insn::SetIvar { .. } | Insn::SetClassVar { .. } | Insn::ArrayExtend { .. }
983-
| Insn::ArrayPush { .. } | Insn::SideExit { .. } | Insn::SetLocal { .. } | Insn::IncrCounter(_)
984-
| Insn::CheckInterrupts { .. } | Insn::GuardBlockParamProxy { .. } | Insn::IncrCounterPtr { .. }
985-
| Insn::StoreField { .. } | Insn::WriteBarrier { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
986-
Insn::Const { val: Const::Value(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
987-
Insn::Const { val: Const::CBool(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
988-
Insn::Const { val: Const::CInt8(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
989-
Insn::Const { val: Const::CInt16(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
990-
Insn::Const { val: Const::CInt32(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
991-
Insn::Const { val: Const::CInt64(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
992-
Insn::Const { val: Const::CUInt8(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
993-
Insn::Const { val: Const::CUInt16(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
994-
Insn::Const { val: Const::CUInt32(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
995-
Insn::Const { val: Const::CUInt64(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
996-
Insn::Const { val: Const::CPtr(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
997-
Insn::Const { val: Const::CDouble(val) } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
998-
Insn::Test { val } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
999-
Insn::IsNil { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1000-
Insn::IsMethodCfunc { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1001-
Insn::IsBitEqual { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1002-
Insn::IsBitNotEqual { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1003-
Insn::BoxBool { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1004-
Insn::BoxFixnum { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1005-
Insn::UnboxFixnum { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1006-
Insn::StringCopy { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1007-
Insn::StringIntern { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1008-
Insn::StringConcat { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1009-
Insn::StringGetbyte { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1010-
Insn::StringSetbyteFixnum { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1011-
Insn::StringAppend { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1012-
Insn::StringAppendCodepoint { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1013-
Insn::ToRegexp { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1014-
Insn::NewArray { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1015-
Insn::ArrayDup { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1016-
Insn::ArrayArefFixnum { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1017-
Insn::ArrayPop { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1018-
Insn::ArrayLength { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1019-
Insn::HashAref { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1020-
Insn::NewHash { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1021-
Insn::HashDup { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1022-
Insn::NewRange { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1023-
Insn::NewRangeFixnum { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1024-
Insn::ObjectAlloc { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1025-
Insn::ObjectAllocClass { class, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1026-
&Insn::CCallWithFrame { return_type, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1027-
Insn::CCall { return_type, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1028-
&Insn::CCallVariadic { return_type, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1029-
Insn::GuardType { val, guard_type, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1030-
Insn::GuardTypeNot { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1031-
Insn::GuardBitEquals { val, expected, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1032-
Insn::GuardShape { val, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1033-
Insn::GuardNotFrozen { recv, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1034-
Insn::GuardLess { left, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1035-
Insn::GuardGreaterEq { left, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1036-
Insn::FixnumAdd { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1037-
Insn::FixnumSub { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1038-
Insn::FixnumMult { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1039-
Insn::FixnumDiv { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1040-
Insn::FixnumMod { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1041-
Insn::FixnumEq { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1042-
Insn::FixnumNeq { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1043-
Insn::FixnumLt { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1044-
Insn::FixnumLe { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1045-
Insn::FixnumGt { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1046-
Insn::FixnumGe { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1047-
Insn::FixnumAnd { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1048-
Insn::FixnumOr { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1049-
Insn::FixnumXor { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1050-
Insn::FixnumLShift { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1051-
Insn::FixnumRShift { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1052-
Insn::PutSpecialObject { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1053-
Insn::SendWithoutBlock { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1054-
Insn::SendWithoutBlockDirect { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1055-
Insn::Send { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1056-
Insn::SendForward { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1057-
Insn::InvokeSuper { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1058-
Insn::InvokeBlock { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1059-
Insn::InvokeBuiltin { return_type, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1060-
Insn::Defined { pushval, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1061-
Insn::DefinedIvar { pushval, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1062-
Insn::GetConstantPath { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1063-
Insn::IsBlockGiven => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1064-
Insn::FixnumBitCheck { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1065-
Insn::ArrayMax { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1066-
Insn::ArrayInclude { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1067-
Insn::DupArrayInclude { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1068-
Insn::ArrayHash { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1069-
Insn::GetGlobal { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1070-
Insn::GetIvar { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1071-
Insn::LoadPC => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1072-
Insn::LoadEC => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1073-
Insn::LoadSelf => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1074-
&Insn::LoadField { return_type, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1075-
Insn::GetSpecialSymbol { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1076-
Insn::GetSpecialNumber { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1077-
Insn::GetClassVar { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1078-
Insn::ToNewArray { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1079-
Insn::ToArray { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1080-
Insn::ObjToString { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1081-
Insn::AnyToString { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1082-
Insn::GetLocal { rest_param: true, .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1083-
Insn::GetLocal { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1084-
// The type of Snapshot doesn't really matter; it's never materialized. It's used only
1085-
// as a reference for FrameState, which we use to generate side-exit code.
1086-
Insn::Snapshot { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
1087-
Insn::IsA { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Any),
975+
Insn::Const { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
976+
Insn::Param => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
977+
Insn::StringCopy { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
978+
Insn::NewArray { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
979+
// NewHash's operands may be hashed and compared for equalityEffect::from_bits(effect_sets::Any, effect_sets::Allocator), which could have
980+
// side-effects.
981+
// fix newhash which seems to be different than the rest
982+
// Insn::NewHash { elementsEffect::from_bits(effect_sets::Any, effect_sets::Allocator), .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
983+
Insn::ArrayDup { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
984+
Insn::HashDup { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
985+
Insn::Test { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
986+
Insn::Snapshot { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
987+
Insn::FixnumAdd { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
988+
Insn::FixnumSub { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
989+
Insn::FixnumMult { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
990+
Insn::FixnumEq { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
991+
Insn::FixnumNeq { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
992+
Insn::FixnumLt { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
993+
Insn::FixnumLe { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
994+
Insn::FixnumGt { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
995+
Insn::FixnumGe { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
996+
Insn::FixnumAnd { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
997+
Insn::FixnumOr { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
998+
Insn::FixnumXor { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
999+
Insn::FixnumLShift { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1000+
Insn::FixnumRShift { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1001+
Insn::GetLocal { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1002+
Insn::IsNil { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1003+
Insn::LoadPC => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1004+
Insn::LoadEC => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1005+
Insn::LoadSelf => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1006+
Insn::LoadField { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1007+
// Special case, let's fix this
1008+
// Insn::CCall { elidable, .. } => !elidable Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1009+
// Special case, let's fix this one too
1010+
// Insn::CCallWithFrame { elidable, .. } => !elidable Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1011+
Insn::ObjectAllocClass { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1012+
Insn::NewRangeFixnum { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1013+
Insn::StringGetbyte { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1014+
Insn::IsBlockGiven => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1015+
Insn::BoxFixnum { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1016+
Insn::BoxBool { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1017+
Insn::IsBitEqual { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1018+
Insn::IsA { .. } => Effect::from_bits(effect_sets::Any, effect_sets::Allocator),
1019+
_ => Effect::from_bits(effect_sets::Any, effect_sets::Any),
10881020
}
10891021
}
10901022

‎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)