revise UnsafeVarMarker (#1545)

src/codegen.rs

@@ -49,48 +49,6 @@ impl<'a> ConjunctInfo<'a> {
     fn perm_var_offset(&self) -> usize {
         self.has_deep_cut as usize
     }
-
-    fn mark_unsafe_vars(&self, mut unsafe_var_marker: UnsafeVarMarker, code: &mut Code) {
-        if code.is_empty() {
-            return;
-        }
-
-        let mut code_index = 0;
-
-        for phase in 0.. {
-            while code[code_index].is_query_instr() {
-                let query_instr = &mut code[code_index];
-
-                if !unsafe_var_marker.mark_safe_vars(query_instr) {
-                    unsafe_var_marker.mark_phase(query_instr, phase);
-                }
-
-                code_index += 1;
-            }
-
-            if code_index + 1 < code.len() {
-                code_index += 1;
-            } else {
-                break;
-            }
-        }
-
-        code_index = 0;
-
-        for phase in 0.. {
-            while code[code_index].is_query_instr() {
-                let query_instr = &mut code[code_index];
-                unsafe_var_marker.mark_unsafe_vars(query_instr, phase);
-                code_index += 1;
-            }
-
-            if code_index + 1 < code.len() {
-                code_index += 1;
-            } else {
-                break;
-            }
-        }
-    }
 }
 
 #[derive(Clone, Copy, Debug)]
@@ -989,7 +947,8 @@ impl<'b> CodeGenerator<'b> {
         let iter = ChunkedIterator::from_rule_body(p1, clauses);
         self.compile_seq(iter, &conjunct_info, &mut code, false)?;
 
-        conjunct_info.mark_unsafe_vars(unsafe_var_marker, &mut code);
+        unsafe_var_marker.mark_unsafe_instrs(&mut code);
+
         self.compile_cleanup(&mut code, &conjunct_info, clauses.last().unwrap_or(p1));
 
         Ok(code)
@@ -1013,7 +972,7 @@ impl<'b> CodeGenerator<'b> {
             }
         }
 
-        UnsafeVarMarker::from_safe_vars(safe_vars)
+        UnsafeVarMarker::from_fact_vars(safe_vars)
     }
 
     pub(crate) fn compile_fact(&mut self, term: &Term) -> Result<Code, CompilationError> {

src/fixtures.rs

@@ -253,68 +253,163 @@ impl<'a> VariableFixtures<'a> {
 
 #[derive(Debug)]
 pub(crate) struct UnsafeVarMarker {
-    pub(crate) unsafe_vars: IndexMap<RegType, usize>,
+    pub(crate) unsafe_perm_vars: IndexMap<usize, usize>,
-    pub(crate) safe_vars: IndexSet<RegType>,
+    pub(crate) unsafe_temp_vars: IndexSet<usize>,
+    pub(crate) safe_perm_vars: IndexSet<usize>,
+    pub(crate) safe_temp_vars: IndexSet<usize>,
 }
 
 impl UnsafeVarMarker {
     pub(crate) fn new() -> Self {
         UnsafeVarMarker {
-            unsafe_vars: IndexMap::new(),
+            unsafe_perm_vars: IndexMap::new(),
-            safe_vars: IndexSet::new(),
+            unsafe_temp_vars: IndexSet::new(),
+            safe_perm_vars: IndexSet::new(),
+            safe_temp_vars: IndexSet::new(),
         }
     }
 
-    pub(crate) fn from_safe_vars(safe_vars: IndexSet<RegType>) -> Self {
+    pub(crate) fn from_fact_vars(safe_vars: IndexSet<RegType>) -> Self {
-        UnsafeVarMarker {
+        let mut unsafe_var_marker = Self::new();
-            unsafe_vars: IndexMap::new(),
+
-            safe_vars,
+        for r in safe_vars {
+            unsafe_var_marker.mark_var_as_safe(r);
+        }
+
+        unsafe_var_marker
+    }
+
+    fn mark_var_as_safe(&mut self, r: RegType) {
+        match r {
+            RegType::Temp(t) => {
+                self.safe_temp_vars.insert(t);
+            }
+            RegType::Perm(p) => {
+                self.safe_perm_vars.insert(p);
+            }
+        };
+    }
+
+    fn mark_var_as_unsafe(&mut self, r: RegType, phase: usize) {
+        match r {
+            RegType::Temp(t) => {
+                self.unsafe_temp_vars.insert(t);
+            }
+            RegType::Perm(p) => {
+                self.unsafe_perm_vars.insert(p, phase);
+            }
         }
     }
 
-    pub(crate) fn mark_safe_vars(&mut self, query_instr: &Instruction) -> bool {
+    fn mark_safe_vars(&mut self, query_instr: &Instruction) -> bool {
         match query_instr {
             &Instruction::PutVariable(r @ RegType::Temp(_), _) |
             &Instruction::SetVariable(r) => {
-                self.safe_vars.insert(r);
+                self.mark_var_as_safe(r);
                 true
             }
             _ => false,
         }
     }
 
-    pub(crate) fn mark_phase(&mut self, query_instr: &Instruction, phase: usize) {
+    fn mark_phase(&mut self, query_instr: &Instruction, phase: usize) {
         match query_instr {
             &Instruction::PutValue(r @ RegType::Perm(_), _) |
             &Instruction::SetValue(r) => {
-                let p = self.unsafe_vars.entry(r).or_insert(0);
+                self.mark_var_as_unsafe(r, phase);
-                *p = phase;
             }
             _ => {}
         }
     }
 
-    pub(crate) fn mark_unsafe_vars(&mut self, query_instr: &mut Instruction, phase: usize) {
+    fn mark_unsafe_perm_vars(&mut self, query_instr: &mut Instruction, phase: usize) {
         match query_instr {
-            &mut Instruction::PutValue(RegType::Perm(i), arg) => {
+            &mut Instruction::PutValue(RegType::Perm(p), arg) => {
-                if let Some(p) = self.unsafe_vars.swap_remove(&RegType::Perm(i)) {
+                if let Some(ph) = self.unsafe_perm_vars.swap_remove(&p) {
-                    if p == phase {
+                    if ph == phase {
-                        *query_instr = Instruction::PutUnsafeValue(i, arg);
+                        *query_instr = Instruction::PutUnsafeValue(p, arg);
-                        self.safe_vars.insert(RegType::Perm(i));
+                        self.safe_perm_vars.insert(p);
                     } else {
-                        self.unsafe_vars.insert(RegType::Perm(i), p);
+                        self.unsafe_perm_vars.insert(p, ph);
                     }
                 }
             }
-            &mut Instruction::SetValue(r) => {
+            &mut Instruction::SetValue(r @ RegType::Perm(p)) if !self.safe_perm_vars.contains(&p) => {
-                if !self.safe_vars.contains(&r) {
+                *query_instr = Instruction::SetLocalValue(r);
-                    *query_instr = Instruction::SetLocalValue(r);
 
-                    self.safe_vars.insert(r);
+                self.safe_perm_vars.insert(p);
-                    self.unsafe_vars.remove(&r);
+                self.unsafe_perm_vars.remove(&p);
-                }
             }
             _ => {}
         }
     }
+
+    fn mark_unsafe_temp_vars(&mut self, query_instr: &mut Instruction) {
+        match query_instr {
+            &mut Instruction::SetValue(r @ RegType::Temp(t)) if !self.safe_temp_vars.contains(&t) => {
+                *query_instr = Instruction::SetLocalValue(r);
+
+                self.safe_temp_vars.insert(t);
+                self.unsafe_temp_vars.remove(&t);
+            }
+            _ => {
+            }
+        }
+    }
+
+    fn clear_temp_vars(&mut self) {
+        self.safe_temp_vars.clear();
+        self.unsafe_temp_vars.clear();
+    }
+
+    pub(crate) fn mark_unsafe_instrs(&mut self, code: &mut Code) {
+        if code.is_empty() {
+            return;
+        }
+
+        let mut code_index = 0;
+
+        for phase in 0.. {
+            while code[code_index].is_query_instr() {
+                let query_instr = &mut code[code_index];
+
+                if !self.mark_safe_vars(query_instr) {
+                    self.mark_phase(query_instr, phase);
+                    self.mark_unsafe_temp_vars(query_instr);
+                }
+
+                code_index += 1;
+            }
+
+            while code_index < code.len() && !code[code_index].is_query_instr() {
+                code_index += 1;
+            }
+
+            self.clear_temp_vars();
+
+            if code_index >= code.len() {
+                break;
+            }
+        }
+
+        code_index = 0;
+
+        for phase in 0.. {
+            while code[code_index].is_query_instr() {
+                let query_instr = &mut code[code_index];
+                self.mark_unsafe_perm_vars(query_instr, phase);
+                code_index += 1;
+            }
+
+            // ensure phase->instruction assignments match those of
+            // the previous for loop.
+            while code_index < code.len() && !code[code_index].is_query_instr() {
+                code_index += 1;
+            }
+
+            if code_index >= code.len() {
+                break;
+            }
+        }
+    }
 }