[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: compound operators
From: |
Jaroslav Hajek |
Subject: |
Re: compound operators |
Date: |
Thu, 8 May 2008 21:48:13 +0200 |
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