On Sun, Aug 10, 2014 at 11:17 PM, Prathamesh Kulkarni
<bilbotheelffri...@gmail.com> wrote:
> On Mon, Aug 4, 2014 at 2:13 PM, Richard Biener
> <richard.guent...@gmail.com> wrote:
>> On Sun, Aug 3, 2014 at 6:58 PM, Prathamesh Kulkarni
>> <bilbotheelffri...@gmail.com> wrote:
>>> On Tue, Jul 29, 2014 at 4:29 PM, Richard Biener
>>> <richard.guent...@gmail.com> wrote:
>>>> On Mon, Jul 28, 2014 at 10:02 PM, Prathamesh Kulkarni
>>>> <bilbotheelffri...@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.

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.

Reply via email to