Add value stack in Thread #608
Conversation
test result: FAILED. 71 passed; 17 failed; 2 ignored; 0 measured; 0 filtered out; finished in 3.91s
failures:
block
br
br_if
br_table
call
call_indirect
fac
global
if_
labels
left_to_right
local_tee
loop_
select
switch
unreachable
unwind
failures:
br
br_if
br_table
if_
labels
loop_
switch
unreachable
unwind
test result: FAILED. 79 passed; 9 failed; 2 ignored; 0 measured; 0 filtered out; finished in 5.16s
| fn last_frame_values_cnt(&mut self) -> usize { | ||
| self.frame().labels.iter().map(|label| label.values_cnt).sum() | ||
| } |
There was a problem hiding this comment.
@ia0 The reason why this is not computed on the fly is that I suspect it could be obtained from the side table. Please let me know if this is not the case.
There was a problem hiding this comment.
Yes, I also believe this could be known statically and doesn't need to be optimized until we get the side table computed. In particular, I expect the values_cnt fields to disappear. So all code mentioning values_cnt fields should also disappear.
| fn last_frame_values_cnt(&mut self) -> usize { | ||
| self.frame().labels.iter().map(|label| label.values_cnt).sum() | ||
| } |
There was a problem hiding this comment.
Yes, I also believe this could be known statically and doesn't need to be optimized until we get the side table computed. In particular, I expect the values_cnt fields to disappear. So all code mentioning values_cnt fields should also disappear.
crates/interpreter/src/exec.rs
Outdated
| } | ||
| values.reverse(); | ||
| values | ||
| pop_values(n, || self.pop_value()) |
There was a problem hiding this comment.
I'm not sure why I wrote this function like that, but it seems Vec::split_off would work (it probably didn't exist back then).
crates/interpreter/src/exec.rs
Outdated
| @@ -1058,25 +1064,27 @@ impl<'m> Thread<'m> { | |||
| } | |||
|
|
|||
| fn exit_label(mut self) -> ThreadResult<'m> { | |||
| let values_cnt = self.last_frame_values_cnt(); | |||
There was a problem hiding this comment.
I simplified this a bit. We don't need to compute this because when we enter the if, we know there was exactly one label.
| self.only_pop_values(n) | ||
| } | ||
|
|
||
| fn only_pop_values(&mut self, n: usize) -> Vec<Val> { |
There was a problem hiding this comment.
I expect this function to disappear once values_cnt disappears, since it becomes equivalent to pop_values.
Reverted #605 and keep values separate from locals for now.
#46