On Thu, Jul 10, 2014 at 8:05 PM, Prathamesh Kulkarni
<bilbotheelffri...@gmail.com> wrote:
> Hi,
> I have attempted to add syntax for symbol to denote multiple operators.
>
> I tried it with few bogus patterns and it appears to work.... hopefully -:)
> eg: (bogus pattern):
> (for op in plus minus
> (match_and_simplify
> (op @0 @1)
> (op @0 @0)))
>
> generates following patterns:
> (plus @0 @1) -> (plus @0 @0) // simplify_0
> (plus @0 @1) -> (mult @0 @0) // simplify_1
> (mult @0 @1) -> (plus @0 @0) // simplify_2
> (mult @0 @1) -> (mult @0 @0) // simplify_3
s/mult/minus/? I think it should only generate
(plus @0 @1) -> (plus @0 @0)
(minus @0 @1) -> (minus @0 @0)
to get the what you write we should need to write
(for op1 in plus minus
(for op2 in plus minus
(match_and_simplify
(op1 @0 @1)
(op2 @0 @0))))
> root (0xab6b10), 0, 2
> |--(PLUS_EXPR) (0xab6b30), 1, 1
> |----true (0xab6ba0), 2, 1
> |------true (0xab6c10), 3, 2
> |--------simplify_0 { 0xab6ba0, 0xab6c10, (nil), (nil), } (0xab6c80), 4, 0
> |--------simplify_1 { 0xab6ba0, 0xab6c10, (nil), (nil), } (0xab6d40), 4, 0
> |--(MULT_EXPR) (0xab6d00), 1, 1
> |----true (0xab6d90), 2, 1
> |------true (0xab6e00), 3, 2
> |--------simplify_2 { 0xab6d90, 0xab6e00, (nil), (nil), } (0xab6e70), 4, 0
> |--------simplify_3 { 0xab6d90, 0xab6e00, (nil), (nil), } (0xab6f30), 4, 0
>
> * Changes to rest of the code:
> a) commutating patterns was interfering with this, because
> parse_match_and_simplify, immediately commutated match operand. Symbol
> should be replaced by operators before commutating. This required
> adjusting simplify (back to operand *match), and commutating is done
> in new function lower_commutative. Ideally this should be done during
> insertion in decision tree ?
Yes, commutating should be done by inserting a pattern multiple
times with adjusted AST walk order.
> b) adjustments required to e_operation constructor, so it doesn't
> call fatal, when it does not find id to be in the hash table.
Eventually just temporarily insert a new operator in the hash table
when parsing comes along a (for OP ...? That is, add a new kind,
USER and just re-use base->id?
> * Caveats
> a) new e_operation constructor taking id_base * argument. Not sure
> if that's required.
> b) e_operation::user_id denotes user-defined identifier (<opname>),
> a rather apologetic name ...
> c) Similar to commutate(), replace_user_id() does not clone AST's.
> So we have multiple AST's sharing same nodes.
I wonder if we want to parse the 'for' into an AST node instead,
thus not expand it on-the-fly. Applying could then happen during
DT insertion. Or as a separate lowering like you introduce
lower_commutative - that looks cleaner.
Or if we can simply parse the match-and-simplify multiple times, with
the for op replaces, using _cpp_backup_tokens. Ok, no - that
sounds like too much of a hack.
> * add multiple symbols ?
> should we have
> (for <opname> in operator-list1, <opname2> in operator-list2
> (match_and_simplify
> ...))
> or have nested for ?
> (for <opname> in operator-list1
> (for <opname2> in operator-list2
> (match_and_simplify
> ....)))
It depends on the desired semantics. It's a lot clearer with
single opname for only (but then we eventually need nested for).
> * we don't detect functions with wrong arguments
> for example, we dont give error on:
> (built_in_sqrt @0 @1)
> I guess that's because we don't have an easy way to figure out
> number of arguments a function expects ?
> (is there a built-in equivalent of tree_code_length[] ?)
Yeah, the function stuff is still very simplistic and no, there isn't
any easy way of extracting the number of expected arguments
from builtins.def (it's encoded in the type). The easiest way
would be to change builtins.def to add a number-of-args argument ...
Let's just defer that issue.
As for the patch, we shouldn't do the cartesian_product - that's
hardly ever what the user intends and it means there isn't any
way of repeating the same pattern for multiple operators.
A common use for 'for' would be (for OP in ne eq (...)) as most
equality foldings are valid for ne and eq. Another is
for the various division kinds we have - trunc_div ceil_div floor_div
and round_div (same for mod):
(for op in trunc_div ceil_div floor_div round_div
(match_and_simplify
(op @0 integer_onep)
@0))
(good example for match.pd to exercise the code)
Can you fix the cartesian product thing (it should only simplify the patch)?
Thanks,
Richard.
> * genmatch.c (e_operation::e_operation): New constructor.
> (e_operation::user_id): New member.
> (e_operation::get_op): New member function.
> (simplify::matchers): Remove.
> (simplify::match): New member.
> (lower_commutative): New function.
> (check_operator): Likewise.
> (replace_user_id): Likewise.
> (decision_tree::insert): Adjust to changes in simplify.
> (eat_ident): New function.
> (parse_expr): Call to check_operator.
> (parse_for): New function.
> (main): Add calls to parse_for, lower_commutative.
>
> Thanks and Regards,
> Prathamesh
