diff mbox

[GIMPLE,FE] Handle abs_expr

Message ID CAAgBjMnu8pMAtuXA72b7rat3-TZHf5=o8=CadE+xVbCgdAwTYw@mail.gmail.com
State New
Headers show

Commit Message

Prathamesh Kulkarni Feb. 8, 2017, 11:07 a.m. UTC 
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.

Comments

Richard Biener Feb. 8, 2017, 11:54 a.m. UTC | #1 
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.

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.
>
diff mbox

Patch

diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c
index e167e42..942597a 100644
--- a/gcc/c/gimple-parser.c
+++ b/gcc/c/gimple-parser.c
@@ -61,7 +61,7 @@  static bool c_parser_gimple_compound_statement (c_parser *, gimple_seq *);
 static void c_parser_gimple_label (c_parser *, gimple_seq *);
 static void c_parser_gimple_statement (c_parser *, gimple_seq *);
 static struct c_expr c_parser_gimple_binary_expression (c_parser *);
-static struct c_expr c_parser_gimple_unary_expression (c_parser *);
+static struct c_expr c_parser_gimple_unary_expression (c_parser *, bool called_from_stmt_parser = false);
 static struct c_expr c_parser_gimple_postfix_expression (c_parser *);
 static struct c_expr c_parser_gimple_postfix_expression_after_primary (c_parser *,
 								       location_t,
@@ -323,7 +323,8 @@  c_parser_gimple_statement (c_parser *parser, gimple_seq *seq)
     }
 
   /* Unary expression.  */
-  switch (c_parser_peek_token (parser)->type)
+  enum cpp_ttype toktype = c_parser_peek_token (parser)->type;
+  switch (toktype)
     {
     case CPP_KEYWORD:
       if (c_parser_peek_token (parser)->keyword != RID_REALPART
@@ -336,7 +337,12 @@  c_parser_gimple_statement (c_parser *parser, gimple_seq *seq)
     case CPP_COMPL:
     case CPP_NOT:
     case CPP_MULT: /* pointer deref */
-      rhs = c_parser_gimple_unary_expression (parser);
+    case CPP_NAME:
+      rhs = c_parser_gimple_unary_expression (parser, true);
+      if (toktype == CPP_NAME
+	  && rhs.value == NULL)
+	break;
+
       if (rhs.value != error_mark_node)
 	{
 	  assign = gimple_build_assign (lhs.value, rhs.value);
@@ -537,11 +543,11 @@  c_parser_gimple_binary_expression (c_parser *parser)
      unary-operator gimple-postfix-expression
 
    unary-operator: one of
-     & * + - ~
+     & * + - ~ abs_expr
 */
 
 static c_expr
-c_parser_gimple_unary_expression (c_parser *parser)
+c_parser_gimple_unary_expression (c_parser *parser, bool called_from_stmt_parser)
 {
   struct c_expr ret, op;
   location_t op_loc = c_parser_peek_token (parser)->location;
@@ -599,6 +606,23 @@  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 if (called_from_stmt_parser)
+	    {
+	      ret.value = NULL;
+	      return ret;
+	    }
+	  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..c75ea67
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/gimplefe-25.c
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fgimple -fdump-tree-ssa-gimple" } */
+
+int __GIMPLE foo(int a)
+{
+  int t1;
+  t1_1 = __ABS a;
+  return t1_1;
+}
+
+/* { dg-final { scan-tree-dump "__ABS a" "ssa" } } */