Re: compound operators

[Jaroslav Hajek]

On Thu, May 8, 2008 at 5:46 PM, John W. Eaton <address@hidden> wrote:
> On  7-May-2008, Jaroslav Hajek wrote:
>
> | I've made first attempt to elaborate on the idea John W. Eaton gave on
> | this list a while ago: having Octave's parser to recognize expressions
> | like `expr1' * expr2' as special, to allow more efficient mapping of
> | operations onto BLAS routines.
> |
> | The initial commit is in my public repo:
> | https://tw-math.de/highegg
>
> OK, this looks like an excellent start.
>
> In functions like
>
>  static octave_value::unary_op
>  strip_trans_herm (tree_expression *&exp)
>  {
>    if (exp->is_unary_expression ())
>      {
>        tree_unary_expression *uexp =
>          dynamic_cast<tree_unary_expression *> (exp);
>        octave_value::unary_op op = uexp->op_type ();
>        if (op == octave_value::op_transpose
>            || op == octave_value::op_hermitian)
>          {
>            exp = uexp->operand ();
>          }
>        else
>          {
>            op = octave_value::unknown_unary_op;
>          }
>        return op;
>      }
>    else
>      return octave_value::unknown_unary_op;
>  }
>
> it looks like there will be a memory leak if you replace exp (which
> was dynamically allocated by the parser) with another part of the
> parse tree.  So maybe this should be
>
>  static octave_value::unary_op
>  strip_trans_herm (tree_expression *&exp)
>  {
>    octave_value::unary_op retval = octave_value::unknown_unary_op;
>
>    if (exp->is_unary_expression ())
>      {
>        tree_unary_expression *uexp =
>          dynamic_cast<tree_unary_expression *> (exp);
>
>        octave_value::unary_op op = uexp->op_type ();
>
>        if (op == octave_value::op_transpose
>            || op == octave_value::op_hermitian)
>          {
>            tree_expression *tmp = exp;
>            exp = uexp->operand ();
>            delete tmp;
>
>            retval = op;
>          }
>      }
>
>    return retval
>  }
>
> ?
>

I hope not. The tree_compound_binary_expression class has a slightly
different philosophy:
it does not "own" the stripped operands, just stores pointers to them
(note that it has no destructor). The orginal operands is still owned
by the parent tree_binary_expression object.

I thought this would be the best design - the compound operator needs
not be visible to printing code (we want the code to print normally
like `A'*B') or breakpoints, so tree-print-code and tree-breakpoint
need not care. The original expression is retained, because it may
still be useful. For example, we may later want to implement a
simplification that
trace (A*B) evaluates as sum((A.*B)(:)) (this is probably not of much
use, but demonstrates the matter). With my approach, trace (A'*B) will
be properly simplified, because the A'*B expression can still be
identified as a multiplication.

This is also the reason why compound_binary_op is a separate enum. In
octave_value and octave_value_typeinfo, I have used overloading of the
existing xxx_binary_op functions if possible, because that allows to
reuse some of the registration macros (and some of the overloaded
functions are simply duplicated - perhaps templates may later be
employed instead, although it didn't seem worth the trouble).

> Also, if you add a new parse tree class, you need to add it to the
> tree_walker class in pt-walk.h and then create corresponding visitor
> functions in all the classes that can walk the tree.  Currently, those
> are in the pt-pr-code, pt-bp, and pt-check files.
>
> jwe
>

Explained above - I didn't consider it necessary, as no other code
needs to know about the simplified expression other than the "virtual
constructor" - maybe_compound_binary_expression in pt-cbinop.h.

Do you see any potential problems with this approach?

regards,

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