From 5f2e37e05c3ffc27dcf6cd14a65cd28046a57a2c Mon Sep 17 00:00:00 2001
From: Jakob Stoklund Olesen <jolesen@mozilla.com>
Date: Mon, 13 Mar 2017 14:07:14 -0700
Subject: [PATCH] Add take_value_list and put_value_list methods.

Any code that needs to manipulate a variable argument list on an
instruction will need to remove the instruction's value list first,
change the list, and then put it back on the instruction. This is
required to avoid fighting the borrow checker over mutable locks on the
DataFlowGraph and its value list pool.

Add a generated InstructionData::take_value_list() method which lifts
out and existing value list and returns it, levaing an empty list in its
place, like Option::take() does it.

Add a generated InstructionData::put_value_list() which puts it back,
verifying that no existing value list is overwritten.
---
 lib/cretonne/meta/gen_instr.py  | 41 +++++++++++++++++++++++++++++++++
 lib/cretonne/src/entity_list.rs |  8 +++++++
 2 files changed, 49 insertions(+)

diff --git a/lib/cretonne/meta/gen_instr.py b/lib/cretonne/meta/gen_instr.py
index c05cb51078..e6eaa82c32 100644
--- a/lib/cretonne/meta/gen_instr.py
+++ b/lib/cretonne/meta/gen_instr.py
@@ -111,6 +111,8 @@ def gen_instruction_data_impl(fmt):
     - `pub fn second_result_mut(&mut self) -> Option<&mut PackedOption<Value>>`
     - `pub fn arguments(&self, &pool) -> &[Value]`
     - `pub fn arguments_mut(&mut self, &pool) -> &mut [Value]`
+    - `pub fn take_value_list(&mut self) -> Option<ValueList>`
+    - `pub fn put_value_list(&mut self, args: ValueList>`
     """
 
     # The `opcode` and `first_type` methods simply read the `opcode` and `ty`
@@ -221,6 +223,45 @@ def gen_instruction_data_impl(fmt):
                 """)
         gen_arguments_method(fmt, True)
 
+        fmt.doc_comment(
+                """
+                Take out the value list with all the value arguments and return
+                it.
+
+                This leaves the value list in the instruction empty. Use
+                `put_value_list` to put the value list back.
+                """)
+        with fmt.indented(
+                'pub fn take_value_list(&mut self) -> Option<ValueList> {',
+                '}'):
+            with fmt.indented('match *self {', '}'):
+                for f in InstructionFormat.all_formats:
+                    n = 'InstructionData::' + f.name
+                    if f.has_value_list:
+                        fmt.line(
+                            n + ' { ref mut args, .. } => Some(args.take()),')
+                fmt.line('_ => None,')
+
+        fmt.doc_comment(
+                """
+                Put back a value list.
+
+                After removing a value list with `take_value_list()`, use this
+                method to put it back. It is required that this instruction has
+                a format that accepts a value list, and that the existing value
+                list is empty. This avoids leaking list pool memory.
+                """)
+        with fmt.indented(
+                'pub fn put_value_list(&mut self, vlist: ValueList) {', '}'):
+            with fmt.indented('let args = match *self {', '};'):
+                for f in InstructionFormat.all_formats:
+                    n = 'InstructionData::' + f.name
+                    if f.has_value_list:
+                        fmt.line(n + ' { ref mut args, .. } => args,')
+                fmt.line('_ => panic!("No value list: {:?}", self),')
+            fmt.line('assert!(args.is_empty(), "Value list already in use");')
+            fmt.line('*args = vlist;')
+
 
 def collect_instr_groups(isas):
     seen = set()
diff --git a/lib/cretonne/src/entity_list.rs b/lib/cretonne/src/entity_list.rs
index 2b8ebb783e..64a0a4c830 100644
--- a/lib/cretonne/src/entity_list.rs
+++ b/lib/cretonne/src/entity_list.rs
@@ -47,6 +47,7 @@
 //! reserved for the empty list which isn't allocated in the vector.
 
 use std::marker::PhantomData;
+use std::mem;
 
 use entity_map::EntityRef;
 
@@ -273,6 +274,13 @@ impl<T: EntityRef> EntityList<T> {
         self.index = 0;
     }
 
+    /// Take all elements from this list and return them as a new list. Leave this list empty.
+    ///
+    /// This is the equivalent of `Option::take()`.
+    pub fn take(&mut self) -> EntityList<T> {
+        mem::replace(self, Default::default())
+    }
+
     /// Appends an element to the back of the list.
     pub fn push(&mut self, element: T, pool: &mut ListPool<T>) {
         let idx = self.index as usize;