Re: tree walker evaluator

[Jaroslav Hajek]

On Wed, Feb 4, 2009 at 7:18 AM, John W. Eaton <address@hidden> wrote:
> On  7-Jan-2009, John W. Eaton wrote:
>
> | On  7-Jan-2009, David Bateman wrote:
> |
> | | I notice this patch also comments out the MAYBE_DO_BREAKPOINT macros.
> | | Does that mean that breakpoints won't be correctly accepted?
> | | Unfortunately "make check" won't spot that type of regression with this
> | | change.
> |
> | Yes, as it is currently written, that macro won't work outside the
> | tree_*::eval member functions.  I'll see if I can make a debugging
> | evaluator work before I check in the changes.  Thinking about this
> | now, should it be possible to do debugging and profiling at the same
> | time?
>
> I checked in the following change:
>
>  http://hg.savannah.gnu.org/hgweb/octave/rev/73c4516fae10
>
> This is a fairly large modification to the interpreter (it touches 103
> files) but I think it is better than what we had before and it passes
> all the tests that were passing before the change, so I think it is
> working correctly and is reasonably safe to commit now.
>
> I think the debugger is now much improved (hey, I might even be able
> to use it now :-).  Setting breakpoints appears to be much more
> reliable and the implementation of dbstep is far simpler and seems to
> work better than before.  Now you only step by statements, not
> individual subexpressions, "dbstep in" and "dbstep out" should work
> correctly, and "dbstep N" will not step past the end of the current
> function.  When a breakpoint is reached, the source line is printed
> before the "debug> " prompt is displayed.
>
> The rvalue methods have been overhauled a bit.  Previously, we had
>
>  octave_value_list rvalue (int nargout);
>
> which was used to evaluate expressions that could produce any number
> of output values, and
>
>  octave_value rvalue (void);
>
> which was used to evaluate expressions that were only supposed to
> produce one output value.  But there was some confusion about how this
> was used.
>
> Now we have
>
>  octave_value_list rvalue (int nargout);
>
> which is the same as before, and
>
>  octave_value rvalue1 (int nargout = 1);
>
> which is used in places that evaluate expressions but will use only
> one value, but in the given context may have nargout set to any
> value (sometimes zero).  I'm not sure there were any cases where
> Octave was producing incorrect results with the old functions, but
> with the new versions the internals seem a little more
> straightforward to me.
>
> I was planning to have a separate tree_debugger class derived from the
> tree_evaluator class and that would replace some of the functions in
> the tree_evaluator class to do the debugging stuff before calling the
> corresponding fucntion from the tree_evaluator class.  For example:
>
>  void
>  tree_debugger::visit_statement (tree_statement& stmt)
>  {
>    // check to see if breakpoint is set and debugging things...
>    ...
>    ...
>    ...
>
>    tree_evaluator::visit_statement (stmt);
>  }
>
> Although it is probably possible to make this work, it did not seem
> worth the effort, and I wasn't sure I could correctly implement
> switching between the normal and debugging evaluators at arbitrary
> times.  So I decided to simply check a flag in the tree_evaluator
> methods that need to do debugging things.  I don't think this adds
> much overhead and it allows reliable switching between the normal and
> debugging evaluators.
>
> Comments?  Bug reports?
>

It's nice to have the walker separated. I've removed two left-over
obsolete declarations and a local method that you've probably
overlooked (pt-loop.h and pt-loop.cc).
I also inspected the code for looping over arrays and replaced the
complicated per-type casing using DO_ND_LOOP with a single generic
branch using do_index_op. Thus, iterating over logical and
single-precision arrays now works correctly (does not convert to
double) and the code seems a lot simpler to me. Further, since
do_index_op normally uses the underlying Array<T> indexing machinery,
this will make such loops automatically benefit from shallow slicing,
making them faster if the body is cheap.

The downside is that this somewhat slows very simple loops over row vectors.
Perhaps some specializations would be appropriate for simple row
vectors and row strings?

regards

-- 
RNDr. Jaroslav Hajek
computing expert
Aeronautical Research and Test Institute (VZLU)
Prague, Czech Republic
url: www.highegg.matfyz.cz