This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [GIMPLE FE] Handle abs_expr
- From: Richard Biener <rguenther at suse dot de>
- To: Prathamesh Kulkarni <prathamesh dot kulkarni at linaro dot org>
- Cc: gcc Patches <gcc-patches at gcc dot gnu dot org>, "Joseph S. Myers" <joseph at codesourcery dot com>
- Date: Wed, 8 Feb 2017 13:16:17 +0100 (CET)
- Subject: Re: [GIMPLE FE] Handle abs_expr
- Authentication-results: sourceware.org; auth=none
- References: <CAAgBjMnmmAZ1=PkPnTpt1PHtX5WiXongK=PoMKK7aFMXTcjvHQ@mail.gmail.com> <alpine.LSU.2.20.1702071525300.19968@zhemvz.fhfr.qr> <CAAgBjMnu8pMAtuXA72b7rat3-TZHf5=o8=CadE+xVbCgdAwTYw@mail.gmail.com> <alpine.LSU.2.20.1702081253270.19968@zhemvz.fhfr.qr> <CAAgBjMmE9QdJ2EohAqtLBxSC3zk=cfSLF6M=6A8nQmBvG-TRBA@mail.gmail.com>
On Wed, 8 Feb 2017, Prathamesh Kulkarni wrote:
> On 8 February 2017 at 17:24, Richard Biener <rguenther@suse.de> wrote:
> > On Wed, 8 Feb 2017, Prathamesh Kulkarni wrote:
> >
> >> 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.
> >
> > 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 <rguenther@suse.de>
> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)