Skip to content

Commit 46e432c

Browse files
mthommthom
committed
restore [o|i]ip zeroing to trust and fix '$get_clause_p'/3 (#2238)
1 parent 44a61fa commit 46e432c

File tree

5 files changed

+86
-75
lines changed

5 files changed

+86
-75
lines changed

‎src/lib/builtins.pl

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1239,7 +1239,10 @@
12391239
; ClauseQualifier = Module
12401240
),
12411241
ClauseQualifier:'$clause'(Head, Body),
1242-
'$get_clause_p'(Head, P, Module).
1242+
% ensure '$get_clause_p'/3 is not the last clause so it can
1243+
% recover the choice point of '$clause' if necessary.
1244+
'$get_clause_p'(Head, P, Module),
1245+
true.
12431246

12441247
call_retract(Head, Body, Name, Arity, Module) :-
12451248
findall(P, builtins:call_retract_helper(Head, Body, P, Module), Ps),

‎src/machine/mod.rs

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1010,6 +1010,20 @@ impl Machine {
10101010

10111011
self.machine_st.heap.truncate(target_h);
10121012

1013+
// these registers don't need to be reset here and MUST
1014+
// NOT be (nor in indexed_try! trust_epilogue is an
1015+
// exception, see next paragraph)! oip could be reset
1016+
// without any adverse effects but iip is needed by
1017+
// get_clause_p to find the last executed clause/2 clause.
1018+
1019+
// trust_epilogue must reset these for the sake of
1020+
// subsequent predicates beginning with
1021+
// switch_to_term. get_clause_p copes by checking
1022+
// self.machine_st.b > self.machine.e: if true, it is safe
1023+
// to use self.machine_st.iip; if false, use the choice
1024+
// point left at the top of the stack by '$clause'
1025+
// (specifically its biip value).
1026+
10131027
// self.machine_st.oip = 0;
10141028
// self.machine_st.iip = 0;
10151029
} else {
@@ -1059,13 +1073,8 @@ impl Machine {
10591073
self.machine_st.stack.truncate(b);
10601074
self.machine_st.heap.truncate(target_h);
10611075

1062-
// these registers don't need to be reset here and MUST NOT be
1063-
// (nor in indexed_try to trust_epilogue)! oip could be reset
1064-
// without any adverse effects but iip is needed by
1065-
// get_clause_p to find the last executed clause/2 clause.
1066-
1067-
// self.machine_st.oip = 0;
1068-
// self.machine_st.iip = 0;
1076+
self.machine_st.oip = 0;
1077+
self.machine_st.iip = 0;
10691078
}
10701079

10711080
#[inline(always)]

‎src/machine/preprocessor.rs

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -565,21 +565,6 @@ impl Preprocessor {
565565
}
566566
}
567567

568-
/*
569-
fn try_term_to_query<'a, LS: LoadState<'a>>(
570-
&mut self,
571-
loader: &mut Loader<'a, LS>,
572-
terms: Vec<Term>,
573-
cut_context: CutContext,
574-
) -> Result<TopLevel, CompilationError> {
575-
Ok(TopLevel::Query(self.setup_query(
576-
loader,
577-
terms,
578-
cut_context,
579-
)?))
580-
}
581-
*/
582-
583568
pub(super) fn try_term_to_tl<'a, LS: LoadState<'a>>(
584569
&mut self,
585570
loader: &mut Loader<'a, LS>,
@@ -607,20 +592,4 @@ impl Preprocessor {
607592
}
608593
}
609594
}
610-
611-
/*
612-
fn try_terms_to_tls<'a, I: IntoIterator<Item = Term>, LS: LoadState<'a>>(
613-
&mut self,
614-
loader: &mut Loader<'a, LS>,
615-
terms: I,
616-
) -> Result<VecDeque<TopLevel>, CompilationError> {
617-
let mut results = VecDeque::new();
618-
619-
for term in terms.into_iter() {
620-
results.push_back(self.try_term_to_tl(loader, term)?);
621-
}
622-
623-
Ok(results)
624-
}
625-
*/
626595
}

‎src/machine/stack.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,12 @@ impl Stack {
203203
}
204204
}
205205

206+
pub(crate) fn top(&self) -> usize {
207+
unsafe {
208+
(*self.buf.ptr.get()) as usize - self.buf.base as usize
209+
}
210+
}
211+
206212
pub(crate) fn allocate_or_frame(&mut self, num_cells: usize) -> usize {
207213
let frame_size = OrFrame::size_of(num_cells);
208214

‎src/machine/system_calls.rs

Lines changed: 60 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1176,14 +1176,12 @@ impl Machine {
11761176
CompilationTarget::Module(target) => target,
11771177
};
11781178

1179-
let bp = self
1179+
let mut bp = self
11801180
.indices
11811181
.get_predicate_code_index(atom!("$clause"), 2, module_name)
11821182
.and_then(|idx| idx.local())
11831183
.unwrap();
11841184

1185-
let p = self.machine_st.iip as usize;
1186-
11871185
macro_rules! extract_ptr {
11881186
($ptr: expr) => {
11891187
match $ptr {
@@ -1199,45 +1197,71 @@ impl Machine {
11991197
};
12001198
}
12011199

1202-
match &self.code[bp] {
1203-
Instruction::IndexingCode(ref indexing_code) => {
1204-
let indexing_code_ptr = match &indexing_code[0] {
1205-
&IndexingLine::Indexing(IndexingInstruction::SwitchOnTerm(_, _, c, _, s)) => {
1206-
if key.1 > 0 {
1207-
s
1208-
} else {
1209-
c
1200+
loop {
1201+
match &self.code[bp] {
1202+
Instruction::IndexingCode(ref indexing_code) => {
1203+
let indexing_code_ptr = match &indexing_code[0] {
1204+
&IndexingLine::Indexing(IndexingInstruction::SwitchOnTerm(_, _, c, _, s)) => {
1205+
if key.1 > 0 {
1206+
s
1207+
} else {
1208+
c
1209+
}
12101210
}
1211-
}
1212-
_ => {
1213-
unreachable!()
1214-
}
1215-
};
1211+
_ => {
1212+
unreachable!()
1213+
}
1214+
};
12161215

1217-
let boip = extract_ptr!(indexing_code_ptr);
1216+
let boip = extract_ptr!(indexing_code_ptr);
12181217

1219-
let boip = match &indexing_code[boip] {
1220-
IndexingLine::Indexing(IndexingInstruction::SwitchOnStructure(ref hm)) => {
1221-
boip + extract_ptr!(hm.get(&key).cloned().unwrap())
1222-
}
1223-
IndexingLine::Indexing(IndexingInstruction::SwitchOnConstant(ref hm)) => {
1224-
boip + extract_ptr!(hm.get(&Literal::Atom(key.0)).cloned().unwrap())
1225-
}
1226-
_ => boip,
1227-
};
1218+
let boip = match &indexing_code[boip] {
1219+
IndexingLine::Indexing(IndexingInstruction::SwitchOnStructure(ref hm)) => {
1220+
boip + extract_ptr!(hm.get(&key).cloned().unwrap())
1221+
}
1222+
IndexingLine::Indexing(IndexingInstruction::SwitchOnConstant(ref hm)) => {
1223+
boip + extract_ptr!(hm.get(&Literal::Atom(key.0)).cloned().unwrap())
1224+
}
1225+
_ => boip,
1226+
};
12281227

1229-
match &indexing_code[boip] {
1230-
IndexingLine::IndexedChoice(indexed_choice) => {
1231-
return (
1232-
skeleton.core.clause_clause_locs[p],
1233-
bp + indexed_choice[p].offset(),
1234-
);
1228+
match &indexing_code[boip] {
1229+
IndexingLine::IndexedChoice(indexed_choice) => {
1230+
let p = if self.machine_st.b > self.machine_st.e {
1231+
// this means the last
1232+
// self.machine_st.iip value has yet
1233+
// to be overwritten by the Trust
1234+
// instruction. In this case, return
1235+
// it.
1236+
self.machine_st.iip as usize
1237+
} else {
1238+
// otherwise, read the '$clause'
1239+
// choicepoint from the top of the
1240+
// stack. this is very volatile in
1241+
// that it depends on '$clause'
1242+
// immediately preceding
1243+
// '$get_clause_p', which cannot be
1244+
// the last clause of the retract
1245+
// helper to delay deallocation of its
1246+
// environment frame.
1247+
let clause_b = self.machine_st.stack.top();
1248+
self.machine_st.stack.index_or_frame(clause_b).prelude.biip as usize
1249+
};
1250+
1251+
return (
1252+
skeleton.core.clause_clause_locs[p],
1253+
bp + indexed_choice[p].offset(),
1254+
);
1255+
}
1256+
_ => unreachable!(),
12351257
}
1236-
_ => unreachable!(),
12371258
}
1238-
}
1239-
_ => {
1240-
return (skeleton.core.clause_clause_locs[p], bp);
1259+
&Instruction::RevJmpBy(offset) => {
1260+
bp -= offset;
1261+
}
1262+
_ => {
1263+
return (skeleton.core.clause_clause_locs.back().cloned().unwrap(), bp);
1264+
}
12411265
}
12421266
}
12431267
}

0 commit comments

Comments
 (0)