On Wed, 8 Feb 2017, Prathamesh Kulkarni wrote: > On 8 February 2017 at 17:24, Richard Biener <rguent...@suse.de> wrote: > > On Wed, 8 Feb 2017, Prathamesh Kulkarni wrote: > > > >> On 7 February 2017 at 20:06, Richard Biener <rguent...@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. > > > > I'd rather compare the name twice without any extra parameter passing. > > As said, some bigger refactoring should be done to avoid this in the > > future. > Is this version OK after bootstrap+test ?
Yes. Thanks, Richard. > Thanks, > Prathamesh > > > > Richard. > > > >> 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. > >> > > > > -- > > Richard Biener <rguent...@suse.de> > > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB > > 21284 (AG Nuernberg) > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
