Re: [GIMPLE FE] Handle abs_expr

[Prathamesh Kulkarni <prathamesh dot kulkarni at linaro dot org>]

On 7 February 2017 at 20:06, Richard Biener <rguenther@suse.de> wrote:
> On Tue, 7 Feb 2017, Prathamesh Kulkarni wrote:
>
>> Hi Richard,
>> The attached patch tries to handle ABS_EXPR in gimple-fe.
>> I am not sure if __ABS_EXPR should be parsed as id (like __MEM)
>> or parse it as keyword (like __GIMPLE). Currently in the patch, I
>> parse it as id.
>> Patch passes gimplefe tests, is it OK to commit after bootstrap+test ?
>
> Few comments - the new oper_code argument to unary parsing seems
> superfluous - you check the name again for __ABS_EXPR.  Also I'd
> spell it __ABS, without the _EXPR part.
Thanks for the suggestions. The name is not checked again for
__ABS_EXPR, if oper_code is set to sth
other than ERROR_MARK. oper_code is set only by c_parser_gimple_statement.
However I agree it's ugly since the comparison code is placed in both
the functions.

In the attached patch, I changed it to __ABS and name comparison is
done only within
c_parser_gimple_unary_expression. However it uses an ugly hack to know
it's called from
c_parser_gimple_statement and then backs off from further parsing the
token if it's type is
CPP_NAME and value is not  "__ABS". Not sure if this is good idea either.

Thanks,
Prathamesh
>
> For __MEM we go to binary op parsing and then take the "not binary op"
> route.  I guess some refactoring might clean up things here - not for
> now though.
>
> I'm not sure whether new RID_ keywords would be prefered for this
> kind of stuff.  We added one for __PHI.  Joseph, is the RID_ space
> somehow limited so that we should avoid ending up with, say, up to
> 226 RID_s for GIMPLE (upper estimate taken from the number of
> tree codes we have in tree.def)?  I see currently cpplib puts
> rid_code into an unsigned char and we do already seem to have quite
> some of them (114 if I count correctly).
>
> Thanks,
> Richard.

Attachment: abs-2.diff
Description: Text document