This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: writing patterns


On Mon, Aug 11, 2014 at 5:52 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Mon, Aug 11, 2014 at 1:06 AM, Prathamesh Kulkarni
> <bilbotheelffriend@gmail.com> wrote:
>> On Mon, Aug 11, 2014 at 2:53 AM, Prathamesh Kulkarni
>> <bilbotheelffriend@gmail.com> wrote:
>>> On Tue, Aug 5, 2014 at 3:15 PM, Richard Biener
>>> <richard.guenther@gmail.com> wrote:
>>>> On Mon, Aug 4, 2014 at 4:44 PM, Prathamesh Kulkarni
>>>> <bilbotheelffriend@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);

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]