On Sun, Aug 21, 2016 at 10:35:17PM +0530, Prasad Ghangal wrote:
> Hi all,
> 
> As a part of my gsoc project. I have completed the following tasks:
> 
> * Parsed gimple-expression
> * Parsed gimple-labels
> * Parsed local declaration
> * Parsed gimple-goto statement
> * Parsed gimple-if-else statement
> * Parsed gimple-switch statement
> * Parsed gimple-return statement
> * Parsed gimple-PHI function
> * Parsed gimple ssa-names along with default def
> * Parsed gimple-call
> 
> * Hacked pass manager to add support for startwith (pass-name) to skip
> early opt passes
> * Modified gimple dump for making it parsable
> 
> I am willing to continue work on the project, some TODOs for the projects are:
> 
> * Error handling
> * Parse more gimple syntax
> * Add startwith support for IPA passes
> 
> The complete code of gimple fe project can be found at
> https://github.com/PrasadG193/gcc_gimple_fe
> 
> 
> PFA patch for complete project (rebased for latest trunk revision).
> I have successfully bootstrapped and tested on x86_64-pc-linux-gnu.
> Some testcases failed due to modified gimple dump as expected.
> 
> 
> Thanks,
> Prasad

only some rather minor comments


+++ b/gcc/c/c-parser.c
@@ -59,6 +59,18 @@ along with GCC; see the file COPYING3.  If not see
 #include "gimple-expr.h"
 #include "context.h"
 #include "gcc-rich-location.h"
+#include "tree-vrp.h"

given that you need these headers it might be better to put most of the
gimple parsing in its own file so only what actually needs to know about
this part of the compiler does now about it.

+void
+c_parser_parse_gimple_body (c_parser *parser)
+{
+  bool return_p = false;
+  gimple_seq seq;
+  gimple_seq body;
+  tree stmt = push_stmt_list ();

it would be nice to move the declarations down to their first use.

+      gimple *ret;
+      ret = gimple_build_return (NULL);

there's no reason for a separate declaration and assignment ;)

+  tree block = NULL;
+  block = pop_scope ();

same here, and a number of other places.

+c_parser_gimple_compound_statement (c_parser *parser, gimple_seq *seq)
+{
+  bool return_p = false;
+
+  if (!c_parser_require (parser, CPP_OPEN_BRACE, "expected %<{%>"))
+      return return_p;

return false would work fine.

+
+  if (c_parser_next_token_is (parser, CPP_CLOSE_BRACE))
+    {
+      c_parser_consume_token (parser);
+      goto out;

I don't see the need for the gotos, there's no cleanup in this function.

+  /* gimple PHI expression.  */
+  if (c_parser_next_token_is_keyword (parser, RID_PHI))
+    {
+      c_parser_consume_token (parser);
+
+      if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
+       {
+         return;
+       }
+
+      gcall *call_stmt;
+      tree arg = NULL_TREE;
+      vec<tree> vargs = vNULL;

I think you can use auto_vec here, as is I think this leaks the vectors
storage.

+c_parser_gimple_binary_expression (c_parser *parser, enum tree_code *subcode)

you can skip the explicit 'enum' keyword.

+  struct {
+    /* The expression at this stack level.  */
+    struct c_expr expr;

similar with struct here.

+    /* The precedence of the operator on its left, PREC_NONE at the
+       bottom of the stack.  */
+    enum c_parser_prec prec;
+    /* The operation on its left.  */
+    enum tree_code op;
+    /* The source location of this operation.  */
+    location_t loc;
+  } stack[2];
+  int sp;
+  /* Location of the binary operator.  */
+  location_t binary_loc = UNKNOWN_LOCATION;  /* Quiet warning.  */
+#define POP                                                                  \

it seems like it would be nicer to name the type, and then make this a
function.

+                                       RO_UNARY_STAR);
+       ret.src_range.m_start = op_loc;
+       ret.src_range.m_finish = finish;
+       return ret;
+      }
+    case CPP_PLUS:
+      if (!c_dialect_objc () && !in_system_header_at (input_location))
+       warning_at (op_loc,
+                   OPT_Wtraditional,
+                   "traditional C rejects the unary plus operator");

does it really make sense to warn about C issues when compiling gimple?

+c_parser_parse_ssa_names (c_parser *parser)
+{
+  tree id = NULL_TREE;
+  c_expr ret;
+  char *var_name, *var_version, *token;
+  ret.original_code = ERROR_MARK;
+  ret.original_type = NULL;
+
+  /* ssa token string.  */
+  const char *ssa_token = NULL;
+  ssa_token = IDENTIFIER_POINTER (c_parser_peek_token (parser)->value);
+  token = new char [strlen (ssa_token)];

I'm not sure I see why you need this copy, and getting rid of it would
mean you don't need to free it.

+  strcpy (token, ssa_token);
+
+  /* seperate var name and version.  */
+  var_version = strrchr (token, '_');
+  if (var_version)
+    {
+      var_name = new char[var_version - token + 1];

you should free this when done with it.

+c_parser_gimple_postfix_expression (c_parser *parser)
+{
+  struct c_expr expr;
+  location_t loc = c_parser_peek_token (parser)->location;;

extra ;

+    case CPP_OBJC_STRING:
+      gcc_assert (c_dialect_objc ());
+      expr.value
+       = objc_build_string_object (c_parser_peek_token (parser)->value);
+      set_c_expr_source_range (&expr, tok_range);
+      c_parser_consume_token (parser);
+      break;

is there a reason to support objc stuff in gimple?

+c_parser_gimple_expr_list (c_parser *parser, bool convert_p,
+                   vec<tree, va_gc> **p_orig_types,
+                   location_t *sizeof_arg_loc, tree *sizeof_arg,
+                   vec<location_t> *locations,
+                   unsigned int *literal_zero_mask)
+{
+  vec<tree, va_gc> *ret;
+  vec<tree, va_gc> *orig_types;
+  struct c_expr expr;
+  location_t loc = c_parser_peek_token (parser)->location;
+  location_t cur_sizeof_arg_loc = UNKNOWN_LOCATION;
+  unsigned int idx = 0;
+
+  ret = make_tree_vector ();
+  if (p_orig_types == NULL)
+    orig_types = NULL;
+  else
+    orig_types = make_tree_vector ();
+
+  if (sizeof_arg != NULL
+      && c_parser_next_token_is_keyword (parser, RID_SIZEOF))
+    cur_sizeof_arg_loc = c_parser_peek_2nd_token (parser)->location;
+  if (literal_zero_mask)
+    c_parser_check_literal_zero (parser, literal_zero_mask, 0);
+  expr = c_parser_gimple_unary_expression (parser);
+  if (convert_p)
+    expr = convert_lvalue_to_rvalue (loc, expr, true, true);
+  ret->quick_push (expr.value);

 That kind of relies on the details of make_tree_vector (), so it seems
 somewhat safer to use vec_safe_push.

+  if (orig_types)
+    orig_types->quick_push (expr.original_type);

same

+c_parser_gimple_declaration (c_parser *parser)
+{
+  struct c_declspecs *specs;
+  struct c_declarator *declarator;
+  specs = build_null_declspecs ();
+  c_parser_declspecs (parser, specs, true, true, true,
+                     true, true, cla_nonabstract_decl);
+  finish_declspecs (specs);
+  bool auto_type_p = specs->typespec_word == cts_auto_type;

is it useful to support auto here in gimple?

+c_parser_gimple_switch_stmt (c_parser *parser, gimple_seq *seq)
+{
+  c_expr cond_expr;
+  tree case_label, label;
+  vec<tree> labels = vNULL;

auto_vec?

+static void
+c_finish_gimple_return (location_t loc, tree retval)
+{
+  tree valtype = TREE_TYPE (TREE_TYPE (current_function_decl));
+
+  /* Use the expansion point to handle cases such as returning NULL
+     in a function returning void.  */
+  source_location xloc = expansion_point_location_if_in_system_header (loc);
+
+  if (TREE_THIS_VOLATILE (current_function_decl))
+    warning_at (xloc, 0,
+               "function declared %<noreturn%> has a %<return%> statement");
+
+  if (!retval)
+    {
+      current_function_returns_null = 1;
+      if ((warn_return_type || flag_isoc99)

I'm not sure what to do about warnings, but checking the language we are
compiling as seems kind of wrong when we're compiling gimple?

@@ -228,6 +228,12 @@ struct GTY(()) function {
   /* GIMPLE body for this function.  */
   gimple_seq gimple_body;
 
+  /* GIMPLEFE pass to start with */
+  opt_pass *pass_startwith = NULL;

I'm guessing you've only compiled in C++11 mode? because I'm pretty sure
you are using a C++11 feature here (the default member value you
assign).

Thanks!

Trev

Reply via email to