[GSoC] replacing op in c_expr
Richard Biener
richard.guenther@gmail.com
Mon Aug 18 12:18:00 GMT 2014
On Sat, Aug 16, 2014 at 3:46 PM, Prathamesh Kulkarni
<bilbotheelffriend@gmail.com> wrote:
> On Mon, Aug 11, 2014 at 4:58 PM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On Sun, Aug 10, 2014 at 11:17 PM, Prathamesh Kulkarni
>> <bilbotheelffriend@gmail.com> wrote:
>>> On Mon, Aug 4, 2014 at 2:13 PM, Richard Biener
>>> <richard.guenther@gmail.com> wrote:
>>>> On Sun, Aug 3, 2014 at 6:58 PM, Prathamesh Kulkarni
>>>> <bilbotheelffriend@gmail.com> wrote:
>>>>> On Tue, Jul 29, 2014 at 4:29 PM, Richard Biener
>>>>> <richard.guenther@gmail.com> wrote:
>>>>>> On Mon, Jul 28, 2014 at 10:02 PM, Prathamesh Kulkarni
>>>>>> <bilbotheelffriend@gmail.com> wrote:
>>>>>>> I am having few issues replacing op in c_expr.
>>>>>>> I thought of following possibilities:
>>>>>>>
>>>>>>> a) create a new vec<cpp_token> vector new_code.
>>>>>>> for each token in code
>>>>>>> {
>>>>>>> if token.type is not CPP_NAME
>>>>>>> new_code.safe_push (token);
>>>>>>> else
>>>>>>> {
>>>>>>> cpp_token new_token =
>>>>>>> ??? create new token of type CPP_NAME
>>>>>>> with contents as name of operator ???
>>>>>>> }
>>>>>>> }
>>>>>>>
>>>>>>> I tried to go this way, but am stuck with creating a new token type.
>>>>>>> i started by:
>>>>>>> cpp_token new_token = token; // get same attrs as token.
>>>>>>> CPP_HASHNODE (new_token.val.node.node)->ident.str = name of operator.
>>>>>>> CPP_HASHNODE (new_token.val.node.node)->ident.len = len of operator name.
>>>>>>> name of operator is obtained from opers[i] in parse_for.
>>>>>>>
>>>>>>> however this does not work because I guess
>>>>>>> new_token = token, shallow copies
>>>>>>> the token (default assignment operator, i didn't find an overloaded version).
>>>>>>>
>>>>>>> b) create new struct c_expr_elem and use
>>>>>>> vec<c_expr_elem> code, instead of vec<cpp_token> code;
>>>>>>>
>>>>>>> sth like:
>>>>>>> struct c_expr_elem
>>>>>>> {
>>>>>>> enum c_expr_elem_type { ID, TOKEN };
>>>>>>> enum c_expr_elem_type type;
>>>>>>>
>>>>>>> union {
>>>>>>> cpp_token token;
>>>>>>> const char *id;
>>>>>>> };
>>>>>>> };
>>>>>>>
>>>>>>> while replacing op, compare token with op, and if it matches,
>>>>>>> create a new c_expr_elem with type = ID, and id = name of operator.
>>>>>>> This shall probably work, but shall require many changes to other parts
>>>>>>> since we change c_expr::code.
>>>>>>>
>>>>>>> I would like to hear any other suggestions.
>>>>>>
>>>>>> Together with the vector of tokens recorded at parse_c_expr time
>>>>>> record a vector of token mappings (op -> plus, op2 -> ...) and do
>>>>>> the replacement at code-generation time where we also special-case
>>>>>> captures.
>>>>>>
>>>>>> Yeah, it's a but unfortunate that c_expr parsing is done the way it
>>>>>> is done....
>>>>> Thanks. I guess we would require a multi-map for this since there can
>>>>> be many operators
>>>>> (op -> [plus, minus], op2 -> [negate]) ?
>>>>
>>>> Well, it would be enough to attach the mapping to c_expr()s after the
>>>> AST lowering when there is at most one? Because obviously
>>>> code-generation cannot know which to replace.
>>>>
>>>>> Unfortunately, I somehow seem to have missed your response and ended up with a
>>>>> hackish way of doing it, although it works. I will soon change that to
>>>>> use token mappings.
>>>>>
>>>>> I mostly followed b), except i made it sub-class of cpp_token, so the
>>>>> other code using c_expr::code
>>>>> (outline_c_expr, c_expr::gen_transform) did not require changes except
>>>>> for special-casing op.
>>>>
>>>> Indeed not too ugly. Still at the point where you replace in the for()
>>>> processing
>>>>
>>>> - operand *result_op = replace_id (s->result, user_id, opers[i]);
>>>> +
>>>> + operand *result_op;
>>>> + if (is_a<c_expr *> (s->result))
>>>> + result_op = replace_op_in_c_expr (s->result, user_id, opers[i]);
>>>> + else
>>>> + result_op = replace_id (s->result, user_id, opers[i]);
>>>> +
>>>> +
>>>>
>>>> it should be "easy" to attach a replacemet vector/map to the c_expr
>>>> and use that duing code-generation.
>>>>
>>>> Note that sub-expressions can also be c_exprs, thus
>>>>
>>>> (match-and-simplify
>>>> (....)
>>>> (plus { ... } @2))
>>>>
>>>> I don't think your patch covers that. That is, you should add
>>>> c_expr handing to replace_id instead.
>>> Thanks, this patch covers that case.
>>> For now, I have still kept the old way, since the change was one-liner.
>>> I will change it after I am done with conditional convert
>>
>> I'll wait for that - the patch introduces extra warnings which will break
>> bootstrap.
>>
> Hi,
> This patch replaces op in c_expr, by using vector in c_expr to record
> <user_id, operator> mapping.
> Sorry for late response.
>
> I needed to clone c_expr, so added clone member function to operand hierarchy.
> Ideally it should be a pure member function (= 0) in operand, however
> for simplicity I have
> put gcc_unreachable (), since I only want it used for c_expr (not
> required so far for other classes).
> Is that okay for now ? Eventually I will implement clone for other classes...
Hmm, I wonder why you didn't go with the simpler approach of simply
copying the replacement vector in replace_id? Like the attached
which I have applied now.
I noted we lack any exercising patterns for this so I added one for
comparison folding also noting another deficiency (we can't
"compute" new operators - somewhat by design as we statically
determine them to simplify further). I have to think about this
(also in the context of manual simplifiers).
2014-08-18 Prathamesh Kulkarni <bilbotheelffriend@gmail.com>
Richard Biener <rguenther@suse.de>
* match-comparison.pd: New file.
* match.pd: Include match-comparison.pd.
* genmatch.c (c_expr): Add vec<id_tab> member.
(replace_id): Handle replacing in c_exprs.
(c_expr::gen_transform): Handle replacing identifiers.
(outline_c_exprs): Likewise.
(parse_for): Replace in if-exprs.
Richard.
> * genmatch.c (operand): New member function clone.
> (c_expr): Override clone.
> (c_expr): New struct id_tab.
> (c_expr): New member ids.
> (simplify::simplify): Adjust to clone c_expr.
> (replace_id): New default parameter replace_c_expr.
> (c_expr::gen_transform): Adjust to replace user-defined
> identifier in c_expr.
> (outline_c_expr): Likewise.
> (parse_for): Likewise.
>
> Thanks,
> Prathamesh
>
>> Thanks,
>> Richard.
>>
>>> * genmatch.c (c_expr::elem): New struct.
>>> (c_expr::code): Change type to vec<c_expr::elem>.
>>> (replace_op_in_c_expr): New function.
>>> (replace_id): Call replace_op_in_c_expr.
>>> (c_expr::gen_transform): Adjust to changes in c_expr.
>>> (outline_c_expr): Likewise.
>>> (parse_c_expr): Likewise.
>>> (parse_for): Call replace_op_in_c_expr.
>>> Thanks,
>>> Prathamesh
>>>>
>>>> Richard.
>>>>
>>>>> * genmatch.c (c_expr::elem): New struct.
>>>>> (c_expr::code): Change type to vec<c_expr::elem>.
>>>>> (replace_op_in_c_expr): New function.
>>>>> (c_expr::gen_transform): Adjust to changes in c_expr.
>>>>> (outline_c_expr): Likewise.
>>>>> (parse_c_expr): Likewise.
>>>>> (parse_for): Call replace_op_in_c_expr.
>>>>>
>>>>> Thanks,
>>>>> Prathamesh
>>>>>
>>>>>>
>>>>>> Richard.
>>>>>>
>>>>>>> Thanks,
>>>>>>> Prathamesh.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: c_expr-my.patch
Type: text/x-patch
Size: 4896 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc/attachments/20140818/0bf05ad4/attachment.bin>
More information about the Gcc
mailing list