From: Mark Date: Thu, 21 Dec 2023 03:45:11 +0000 (-0700) Subject: restore [o|i]ip zeroing to trust and fix '$get_clause_p'/3 (#2238) X-Git-Tag: v0.9.4~64 X-Git-Url: https://git.sagredo.dev/?a=commitdiff_plain;h=46e432c4d9cff6a14e719475b4eebf0d9261b06f;p=scryer-prolog.git restore [o|i]ip zeroing to trust and fix '$get_clause_p'/3 (#2238) --- diff --git a/src/lib/builtins.pl b/src/lib/builtins.pl index 9cb533d1..b24244d6 100644 --- a/src/lib/builtins.pl +++ b/src/lib/builtins.pl @@ -1239,7 +1239,10 @@ call_retract_helper(Head, Body, P, Module) :- ; ClauseQualifier = Module ), ClauseQualifier:'$clause'(Head, Body), - '$get_clause_p'(Head, P, Module). + % ensure '$get_clause_p'/3 is not the last clause so it can + % recover the choice point of '$clause' if necessary. + '$get_clause_p'(Head, P, Module), + true. call_retract(Head, Body, Name, Arity, Module) :- findall(P, builtins:call_retract_helper(Head, Body, P, Module), Ps), diff --git a/src/machine/mod.rs b/src/machine/mod.rs index fccf7d59..431592c8 100644 --- a/src/machine/mod.rs +++ b/src/machine/mod.rs @@ -1010,6 +1010,20 @@ impl Machine { self.machine_st.heap.truncate(target_h); + // these registers don't need to be reset here and MUST + // NOT be (nor in indexed_try! trust_epilogue is an + // exception, see next paragraph)! oip could be reset + // without any adverse effects but iip is needed by + // get_clause_p to find the last executed clause/2 clause. + + // trust_epilogue must reset these for the sake of + // subsequent predicates beginning with + // switch_to_term. get_clause_p copes by checking + // self.machine_st.b > self.machine.e: if true, it is safe + // to use self.machine_st.iip; if false, use the choice + // point left at the top of the stack by '$clause' + // (specifically its biip value). + // self.machine_st.oip = 0; // self.machine_st.iip = 0; } else { @@ -1059,13 +1073,8 @@ impl Machine { self.machine_st.stack.truncate(b); self.machine_st.heap.truncate(target_h); - // these registers don't need to be reset here and MUST NOT be - // (nor in indexed_try to trust_epilogue)! oip could be reset - // without any adverse effects but iip is needed by - // get_clause_p to find the last executed clause/2 clause. - - // self.machine_st.oip = 0; - // self.machine_st.iip = 0; + self.machine_st.oip = 0; + self.machine_st.iip = 0; } #[inline(always)] diff --git a/src/machine/preprocessor.rs b/src/machine/preprocessor.rs index 2681e8c0..b6e82ab7 100644 --- a/src/machine/preprocessor.rs +++ b/src/machine/preprocessor.rs @@ -565,21 +565,6 @@ impl Preprocessor { } } - /* - fn try_term_to_query<'a, LS: LoadState<'a>>( - &mut self, - loader: &mut Loader<'a, LS>, - terms: Vec, - cut_context: CutContext, - ) -> Result { - Ok(TopLevel::Query(self.setup_query( - loader, - terms, - cut_context, - )?)) - } - */ - pub(super) fn try_term_to_tl<'a, LS: LoadState<'a>>( &mut self, loader: &mut Loader<'a, LS>, @@ -607,20 +592,4 @@ impl Preprocessor { } } } - - /* - fn try_terms_to_tls<'a, I: IntoIterator, LS: LoadState<'a>>( - &mut self, - loader: &mut Loader<'a, LS>, - terms: I, - ) -> Result, CompilationError> { - let mut results = VecDeque::new(); - - for term in terms.into_iter() { - results.push_back(self.try_term_to_tl(loader, term)?); - } - - Ok(results) - } - */ } diff --git a/src/machine/stack.rs b/src/machine/stack.rs index 46638bee..a300d5f3 100644 --- a/src/machine/stack.rs +++ b/src/machine/stack.rs @@ -203,6 +203,12 @@ impl Stack { } } + pub(crate) fn top(&self) -> usize { + unsafe { + (*self.buf.ptr.get()) as usize - self.buf.base as usize + } + } + pub(crate) fn allocate_or_frame(&mut self, num_cells: usize) -> usize { let frame_size = OrFrame::size_of(num_cells); diff --git a/src/machine/system_calls.rs b/src/machine/system_calls.rs index 675340a0..3aa4ec73 100644 --- a/src/machine/system_calls.rs +++ b/src/machine/system_calls.rs @@ -1176,14 +1176,12 @@ impl Machine { CompilationTarget::Module(target) => target, }; - let bp = self + let mut bp = self .indices .get_predicate_code_index(atom!("$clause"), 2, module_name) .and_then(|idx| idx.local()) .unwrap(); - let p = self.machine_st.iip as usize; - macro_rules! extract_ptr { ($ptr: expr) => { match $ptr { @@ -1199,45 +1197,71 @@ impl Machine { }; } - match &self.code[bp] { - Instruction::IndexingCode(ref indexing_code) => { - let indexing_code_ptr = match &indexing_code[0] { - &IndexingLine::Indexing(IndexingInstruction::SwitchOnTerm(_, _, c, _, s)) => { - if key.1 > 0 { - s - } else { - c + loop { + match &self.code[bp] { + Instruction::IndexingCode(ref indexing_code) => { + let indexing_code_ptr = match &indexing_code[0] { + &IndexingLine::Indexing(IndexingInstruction::SwitchOnTerm(_, _, c, _, s)) => { + if key.1 > 0 { + s + } else { + c + } } - } - _ => { - unreachable!() - } - }; + _ => { + unreachable!() + } + }; - let boip = extract_ptr!(indexing_code_ptr); + let boip = extract_ptr!(indexing_code_ptr); - let boip = match &indexing_code[boip] { - IndexingLine::Indexing(IndexingInstruction::SwitchOnStructure(ref hm)) => { - boip + extract_ptr!(hm.get(&key).cloned().unwrap()) - } - IndexingLine::Indexing(IndexingInstruction::SwitchOnConstant(ref hm)) => { - boip + extract_ptr!(hm.get(&Literal::Atom(key.0)).cloned().unwrap()) - } - _ => boip, - }; + let boip = match &indexing_code[boip] { + IndexingLine::Indexing(IndexingInstruction::SwitchOnStructure(ref hm)) => { + boip + extract_ptr!(hm.get(&key).cloned().unwrap()) + } + IndexingLine::Indexing(IndexingInstruction::SwitchOnConstant(ref hm)) => { + boip + extract_ptr!(hm.get(&Literal::Atom(key.0)).cloned().unwrap()) + } + _ => boip, + }; - match &indexing_code[boip] { - IndexingLine::IndexedChoice(indexed_choice) => { - return ( - skeleton.core.clause_clause_locs[p], - bp + indexed_choice[p].offset(), - ); + match &indexing_code[boip] { + IndexingLine::IndexedChoice(indexed_choice) => { + let p = if self.machine_st.b > self.machine_st.e { + // this means the last + // self.machine_st.iip value has yet + // to be overwritten by the Trust + // instruction. In this case, return + // it. + self.machine_st.iip as usize + } else { + // otherwise, read the '$clause' + // choicepoint from the top of the + // stack. this is very volatile in + // that it depends on '$clause' + // immediately preceding + // '$get_clause_p', which cannot be + // the last clause of the retract + // helper to delay deallocation of its + // environment frame. + let clause_b = self.machine_st.stack.top(); + self.machine_st.stack.index_or_frame(clause_b).prelude.biip as usize + }; + + return ( + skeleton.core.clause_clause_locs[p], + bp + indexed_choice[p].offset(), + ); + } + _ => unreachable!(), } - _ => unreachable!(), } - } - _ => { - return (skeleton.core.clause_clause_locs[p], bp); + &Instruction::RevJmpBy(offset) => { + bp -= offset; + } + _ => { + return (skeleton.core.clause_clause_locs.back().cloned().unwrap(), bp); + } } } }