Message ID | CAAgBjMmE9QdJ2EohAqtLBxSC3zk=cfSLF6M=6A8nQmBvG-TRBA@mail.gmail.com |
---|---|
State | New |
Headers | show |
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) >
diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c index e167e42..681951c 100644 --- a/gcc/c/gimple-parser.c +++ b/gcc/c/gimple-parser.c @@ -325,6 +325,13 @@ c_parser_gimple_statement (c_parser *parser, gimple_seq *seq) /* Unary expression. */ switch (c_parser_peek_token (parser)->type) { + case CPP_NAME: + { + tree id = c_parser_peek_token (parser)->value; + if (strcmp (IDENTIFIER_POINTER (id), "__ABS") == 0) + goto build_unary_expr; + break; + } case CPP_KEYWORD: if (c_parser_peek_token (parser)->keyword != RID_REALPART && c_parser_peek_token (parser)->keyword != RID_IMAGPART) @@ -336,6 +343,7 @@ c_parser_gimple_statement (c_parser *parser, gimple_seq *seq) case CPP_COMPL: case CPP_NOT: case CPP_MULT: /* pointer deref */ + build_unary_expr: rhs = c_parser_gimple_unary_expression (parser); if (rhs.value != error_mark_node) { @@ -537,7 +545,7 @@ c_parser_gimple_binary_expression (c_parser *parser) unary-operator gimple-postfix-expression unary-operator: one of - & * + - ~ + & * + - ~ abs_expr */ static c_expr @@ -599,6 +607,18 @@ c_parser_gimple_unary_expression (c_parser *parser) default: return c_parser_gimple_postfix_expression (parser); } + case CPP_NAME: + { + tree id = c_parser_peek_token (parser)->value; + if (strcmp (IDENTIFIER_POINTER (id), "__ABS") == 0) + { + c_parser_consume_token (parser); + op = c_parser_gimple_postfix_expression (parser); + return parser_build_unary_op (op_loc, ABS_EXPR, op); + } + else + return c_parser_gimple_postfix_expression (parser); + } default: return c_parser_gimple_postfix_expression (parser); } diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c index 91c839d..0033e97 100644 --- a/gcc/gimple-pretty-print.c +++ b/gcc/gimple-pretty-print.c @@ -323,9 +323,17 @@ dump_unary_rhs (pretty_printer *buffer, gassign *gs, int spc, int flags) break; case ABS_EXPR: - pp_string (buffer, "ABS_EXPR <"); - dump_generic_node (buffer, rhs, spc, flags, false); - pp_greater (buffer); + if (flags & TDF_GIMPLE) + { + pp_string (buffer, "__ABS "); + dump_generic_node (buffer, rhs, spc, flags, false); + } + else + { + pp_string (buffer, "ABS_EXPR <"); + dump_generic_node (buffer, rhs, spc, flags, false); + pp_greater (buffer); + } break; default: diff --git a/gcc/testsuite/gcc.dg/gimplefe-25.c b/gcc/testsuite/gcc.dg/gimplefe-25.c new file mode 100644 index 0000000..d8c44a7 --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-25.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O -fgimple -fdump-tree-ssa-gimple" } */ + +int __GIMPLE() f(int a) +{ + int t0; + t0_1 = __ABS a; + return t0_1; +} + +/* { dg-final { scan-tree-dump "__ABS a" "ssa" } } */