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 Tue, Aug 12, 2014 at 12:47 AM, Prathamesh Kulkarni
<bilbotheelffriend@gmail.com> wrote:
> 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 - 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


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