On Mon, Aug 11, 2014 at 5:52 PM, Richard Biener
<richard.guent...@gmail.com> wrote:
> On Mon, Aug 11, 2014 at 1:06 AM, Prathamesh Kulkarni
> <bilbotheelffri...@gmail.com> wrote:
>> On Mon, Aug 11, 2014 at 2:53 AM, Prathamesh Kulkarni
>> <bilbotheelffri...@gmail.com> wrote:
>>> On Tue, Aug 5, 2014 at 3:15 PM, Richard Biener
>>> <richard.guent...@gmail.com> wrote:
>>>> On Mon, Aug 4, 2014 at 4:44 PM, Prathamesh Kulkarni
>>>> <bilbotheelffri...@gmail.com> wrote:
>>>>> On Mon, Aug 4, 2014 at 2:59 PM, Richard Biener
>>>> No, not changing tree.def but its use as you did in your first proposed
>>>> patch.
>>> I have got it done except for capturing optional converts.
>>> for instance (negate (convert1@0 @1)) does not work.
>>> This is because of insertion in decision tree (insert::operand).
>
> That's because you lower it to
>
> (negate @0@1)
>
> right? a capture with ->what being a capture itself? That's reasonable
> btw.
>
>>> Since captures were always predicate or expressions,
>>> ie, the corresponding dt-node->type was dt_node::DT_OPERAND,
>>> i could hard-code for a search on dt_node::DT_OPERAND.
>>> This changes when capture itself can be captured
>>> (dt_node::DT_TRUE/DT_MATCH),
>>> and there's no easy way right now to obtain corresponding
>>> dt-node from a given operand node.
>>>
>>> Should we keep a mapping (hash_map/std::map) from operand * to dt_node * ?
>>> That will also get rid of dt_node::parent (but we need to then
>>> introduce parent in AST).
>>>
>>> Other cases where convert1, convert2 are not captured work
>>> fine.
>>> (bit_not (convert1 @0) (convert2 @1))
>>> I have attached patch for reference (don't intend to get it committed).
>>
>> I have fixed it in a hackish way in the current patch, so capturing
>> optional convert works..
>> Since we don't know type corresponding to capture in decision tree
>> :DT_TRUE/DT_MATCH (unless
>> we get hold of dt-node corresponding to the capture), I tried both the
>> possibilities.
>
> That looks reasonable to me though I manage to hit the assert with
>
> (simplify
> (plus (convert1@1 (plus@2 INTEGER_CST_P INTEGER_CST_P)) @3)
> (if (useless_type_conversion_p (TREE_TYPE (@1), TREE_TYPE (@2))))
> @2)
>
> when ->what is an expression.
oops, this leads to multiple captures -:)
in this case we have 2-captures: @1 capturing @2 capturing inner plus
expression.
we could have n-captures by introducing arbitrary number of continuous convert1.
for instance:
(plus (convert1@1 (convert1@2 (plus@3 INTEGER_CST_P INTEGER_CST_P))) @4)
which is a 3-captures (@1:@2:@3:plus)
This patch attempts to handle multiple number of captures.
* genmatch.c (decision_tree::insert_operand): Adjust to allow multiple capture.
Thanks,
Prathamesh
>
> I'd also like the various compares against CONVERT1/2 to be not done
> as string compares but using enum tree_code CONVERT1
> equality checks.
>
> I have fixed that (but not the above insertion issue) and committed the
> patch.
>
> Note that I'd still like the syntax to include a question mark, thus
> convert?, convert1? or convert2? should be conditional converts.
> Requires a parser hack though.
hmm, but since we use convert1, convert2 explicitly to denote
conditional convert,
so probably no need for a marker like '?' ?
Thanks,
Prathamesh
>
> Thanks,
> Richard.
>
>> * genmatch.c (enum tree_code): Add new values CONVERT1, CONVERT2.
>> (lower_opt_convert): New function.
>> (lower_opt_convert): New function overloaded.
>> (lower_opt_convert): Likewise.
>> (lower_opt_convert): Likewise.
>> (remove_opt_convert): New function.
>> (has_opt_convert): Likewise.
>> (decision_tree::insert_operand): Adjust for capturing a capture.
>> (parse_for): Reject CONVERT1, CONVERT2.
>> (main): call add_operator.
>> call lower_opt_convert.
>>
>> Thanks,
>> Prathamesh
>>>
>>> Thanks,
>>> Prathamesh
>>>>
>>>> Richard.
>>>>
>>>>> Thanks,
>>>>> Prathamesh
>>>>>>
>>>>>> Richard.
>>>>>>
>>>>>>>
>>>>>>> * genmatch.c (enum tree_code): New value OPT_CONVERT.
>>>>>>> (e_operation): New member unsigned group_index.
>>>>>>> (e_operation::e_operation): Add new default argument.
>>>>>>> Adjust to changes in
>>>>>>> e_operation.
>>>>>>> (remove_opt_convert): New function.
>>>>>>> (has_opt_convert): Likewise.
>>>>>>> (lower_opt_convert): Likewise.
>>>>>>> (lower_opt_convert): New overloaded function.
>>>>>>> (lower_opt_convert): Likewise.
>>>>>>> (parse_expr): Adjust to parse opt_convert.
>>>>>>> (parse_for): Reject opt_convert in opers.
>>>>>>> (main): Call lower_opt_convert.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Prathamesh
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Prathamesh.
>>>>>>>>>
>>>>>>>>> Richard.
>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Prathamesh
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Richard.
>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Prathamesh
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>> Prathamesh
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Extending for in this way avoids treating conversions specially
>>>>>>>>>>>>>> (I don't see an easy way to do very good at that automagically).
>>>>>>>>>>>>>> We
>>>>>>>>>>>>>> need multiple patterns in the decision tree here anyway.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Now guess sth fancy in place of '*' ...
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Lisp style would be less free-form like
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> (for cvt (convert ())
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> where we could use an empty list for the "empty" operator.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Is nested for support already implemented?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> Richard.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> which suggests that we may want to add some type capturing /
>>>>>>>>>>>>>>> matching
>>>>>>>>>>>>>>> so we can maybe write
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> (plus (convert@T (lshift (convert@T2 X) CNT1))
>>>>>>>>>>>>>>> (convert (rshift (convert@T2 X) CNT2)))
>>>>>>>>>>>>>>> (if (/* T2s will be matched automagically */
>>>>>>>>>>>>>>> && TYPE_UNSIGNED (@T)
>>>>>>>>>>>>>>> && TYPE_PRECISION (@T2) > TYPE_PRECISION (@T)
>>>>>>>>>>>>>>> && wi::eq_p (TYPE_PRECISION (@T), wi::add (CNT1, CNT2))))
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> which is less to type and supports requiring matching types.
>>>>>>>>>>>>>>> Maybe
>>>>>>>>>>>>>>> require @T[0-9]+ here thus use @T0 and disallow plain @T. We
>>>>>>>>>>>>>>> could
>>>>>>>>>>>>>>> then also use @T for the implicitely "captured" outermost type
>>>>>>>>>>>>>>> we
>>>>>>>>>>>>>>> refer to as plain 'type' right now.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I suggest to go ahead without a new syntax for now and see if it
>>>>>>>>>>>>>>> gets unwieldingly ugly without first.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> For this week, I have planned:
>>>>>>>>>>>>>>>> a) writing patterns from simplify_rotate
>>>>>>>>>>>>>>>> b) replacing op in c_expr
>>>>>>>>>>>>>>>> c) writing more test-cases.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> If there's anything else you would like me to do, I would be
>>>>>>>>>>>>>>>> happy
>>>>>>>>>>>>>>>> to know.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Just keep an eye open for things like above - easy ways to
>>>>>>>>>>>>>>> reduce
>>>>>>>>>>>>>>> typing for patterns.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Btw, I suggest to split up match.pd by code you converted from.
>>>>>>>>>>>>>>> Thus
>>>>>>>>>>>>>>> for simplify_rotate add
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> match-simplify-rotate.pd
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> with the patterns and do a #include "match-simplify-rotate.pd"
>>>>>>>>>>>>>>> in match.pd. That will make it easier to match the two later.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>> Richard.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>> Prathamesh
Index: genmatch.c
===================================================================
--- genmatch.c (revision 213814)
+++ genmatch.c (working copy)
@@ -1052,7 +1052,7 @@ dt_node::append_simplify (simplify *s, u
dt_node *
decision_tree::insert_operand (dt_node *p, operand *o, dt_operand **indexes, unsigned pos, dt_node *parent)
{
- dt_node *q, *elm;
+ dt_node *q, *elm = 0;
if (o->type == operand::OP_CAPTURE)
{
@@ -1062,38 +1062,42 @@ decision_tree::insert_operand (dt_node *
if (indexes[capt_index] == 0)
{
if (c->what)
+ q = insert_operand (p, c->what, indexes, pos, parent);
+ else
{
- q = insert_operand (p, c->what, indexes, pos, parent);
-
- if (capture *cc = dyn_cast<capture *> (c->what))
- {
- unsigned cc_index = atoi (cc->where);
- dt_operand *match_op = indexes[cc_index];
-
- dt_operand temp (dt_node::DT_TRUE, 0, 0);
- elm = decision_tree::find_node (p->kids, &temp);
+ q = elm = p->append_true_op (parent, pos);
+ goto at_assert_elm;
+ }
+ // get to the last capture
+ for (operand *what = c->what; what && is_a<capture *> (what); c = as_a<capture *> (what), what = c->what)
+ ;
- if (elm == 0)
- {
- dt_operand temp (dt_node::DT_MATCH, 0, match_op);
- elm = decision_tree::find_node (p->kids, &temp);
- }
- }
- else
+ if (c->what == 0)
+ {
+ unsigned cc_index = atoi (c->where);
+ dt_operand *match_op = indexes[cc_index];
+
+ dt_operand temp (dt_node::DT_TRUE, 0, 0);
+ elm = decision_tree::find_node (p->kids, &temp);
+
+ if (elm == 0)
{
- dt_operand temp (dt_node::DT_OPERAND, c->what, 0);
+ dt_operand temp (dt_node::DT_MATCH, 0, match_op);
elm = decision_tree::find_node (p->kids, &temp);
}
-
- gcc_assert (elm);
}
- else
- q = elm = p->append_true_op (parent, pos);
-
- gcc_assert (elm->type == dt_node::DT_TRUE || elm->type == dt_node::DT_OPERAND || elm->type == dt_node::DT_MATCH);
- indexes[capt_index] = static_cast<dt_operand *> (elm);
- return q;
- }
+ else
+ {
+ dt_operand temp (dt_node::DT_OPERAND, c->what, 0);
+ elm = decision_tree::find_node (p->kids, &temp);
+ }
+
+at_assert_elm:
+ gcc_assert (elm);
+ gcc_assert (elm->type == dt_node::DT_TRUE || elm->type == dt_node::DT_OPERAND || elm->type == dt_node::DT_MATCH);
+ indexes[capt_index] = static_cast<dt_operand *> (elm);
+ return q;
+ }
else
{
p = p->append_match_op (indexes[capt_index], parent, pos);
