On Tue, Aug 12, 2014 at 12:47 AM, Prathamesh Kulkarni <bilbotheelffri...@gmail.com> wrote: > 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 - applied. > 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 '?' ? Well, a '?' is so much clearer than a '1' - and being able to write convert? is nicer than convert1 (so we need convert1 and convert2 only to form groups). Well, I give it a quick try in a second ;) Thanks, Richard. > 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
