Re: [gimplefe] Parsing __GIMPLE function body

[Prathamesh Kulkarni <prathamesh dot kulkarni at linaro dot org>]

On 6 June 2016 at 15:49, Richard Biener <richard.guenther@gmail.com> wrote:
> On Mon, Jun 6, 2016 at 11:27 AM, Prasad Ghangal
> <prasad.ghangal@gmail.com> wrote:
>> Hi,
>>
>> This patch parses simple assignment statement
>>
>> int a;
>> void __GIMPLE foo()
>> {
>>   a = 1;
>> }
>>
>> but it does not produce gimple dump. In debugging I found that
>> cfun->gimple_body is not NULL and it contains GIMPLE_ASSIGN statement.
>> Am I missing something ?
>
> This is because the cgraph code does not call gimplify_function_tree
> () on functions
> with a gimple body (undertstandably) but that function produces said dump file.
>
> Now, we don't even get that far as as you call finish_function () the C FE sees
> no DECL_INITIAL on the fndecl (that contains the BLOCK tree), it doesn't even
> finalize the function via the cgraph (cgraph_node::finalize_function ()).
>
> While I believe that in the end we want a custom finish_function for
> the GIMPLE FE
> the immediate issue is the lack of a BLOCK tree which you can try initializing
> to NULL_TREE or an empty scope by simply doing
>
>  DECL_INITIAL (fndecl) = make_node (BLOCK);
>  BLOCK_SUPERCONTEXT (DECL_INITIAL (fndecl)) = fndecl;
>
> that then runs into
>
> #1  0x0000000001798f21 in lower_function_body ()
>     at /space/rguenther/src/svn/GSoC/gcc/gimple-low.c:93
> 93        gcc_assert (gimple_seq_first (body) == gimple_seq_last (body)
> (gdb) l
> 88        gimple *bind;
> 89        gimple *x;
> 90
> 91        /* The gimplifier should've left a body of exactly one statement,
> 92           namely a GIMPLE_BIND.  */
> 93        gcc_assert (gimple_seq_first (body) == gimple_seq_last (body)
> 94                    && gimple_code (gimple_seq_first_stmt (body)) ==
> GIMPLE_BIND);
>
> as GIMPLE lowering expects "high GIMPLE" and thus an outer GIMPLE_BIND.
>
> It might be the time to investigate how to "skip" some early passes, for example
> by first of all declaring what kind of GIMPLE you emit via setting
> cfun->curr_properties - in this case to PROP_gimple_any | PROP_gimple_lcf
>
> Ideally the pass manager would then skip parts of the pipeline that
> are not necessary.
>
> You will notice that a lot of time GCC development is chasing asserts
> / assumptions
> of some part of the compiler and in this process understand GCC some more ;)
Hi,
I was playing a bit with Prasad's patch and got it to work for the
following test-case:
int a;
void __GIMPLE foo()
{
  a = 1;
}

Surprisingly there were very few changes needed for the above test-case
(perhaps more will be needed for a non-trivial case).
I called cgraph_node::finalize_decl(current_function_decl, false)
after parsing was done,
and following hacks were required to get the test-case compiled:

1) Setting empty block scope:
 DECL_INITIAL (current_function_decl) = make_node (BLOCK);
 BLOCK_SUPERCONTEXT (DECL_INITIAL (current_function_decl)) =
current_function_decl;

2) Setting property to PROP_gimple_any however hits the assert in
lower_function_body() since
it expects GIMPLE_BODY.

To workaround that I changed it to:
 if  (! (gimple_seq_first (body) == gimple_seq_last (body)
          && gimple_code (gimple_seq_first_stmt (body)) == GIMPLE_BIND))
    return 0;
which effectively disables pass_lower_cf in gimple mode.

3) After that, it appears middle-end expects a compulsory return stmt,
which is built implicitly
by pass_lower_cf if none exists, but now ICE's since we disable it, so
built it implicitly after parsing gimple body.

4) Added the following code from lower_function_body() to
c_parser_parse_gimple_body (),
not sure if it's really required, I added it because it initialized
different fields of current_function_decl.

  tree block = DECL_INITIAL (current_function_decl);
  BLOCK_SUBBLOCKS (block) = NULL_TREE;
  BLOCK_CHAIN (block) = NULL_TREE;
  TREE_ASM_WRITTEN (block) = 1;

5) Call init_tree_ssa() from C FE since we don't call gimplify_body()
in gimple mode and
init_tree_ssa() is called from gimplify_body().

Extending to basic block should be (hopefully) straight-forward.
I have a silly question - For control-flow  I suppose we can build
"plain" gimple
and let cfg pass take care of building the control flow graph ? Which
makes me wonder how to handle phi's in gimple input ?
create_phi_node() would expect cfg to be built beforehand, so would we
need to build cfg by hand instead of tree-cfg pass ?

Thanks,
Prathamesh
>
> Richard.
>
>>
>>
>> Thanks,
>> Prasad Ghangal
>>
>> On 31 May 2016 at 15:57, Richard Biener <richard.guenther@gmail.com> wrote:
>>> On Mon, May 30, 2016 at 5:15 PM, Prasad Ghangal
>>> <prasad.ghangal@gmail.com> wrote:
>>>> Hi,
>>>>
>>>> As David suggested in his rtlfe patch,
>>>> this patch recognizes __GIMPLE keyword and switches to
>>>> c_parser_parse_gimple_body by providing -fgimple option.
>>>>
>>>>
>>>> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
>>>> index 4568cf6..41a8f05 100644
>>>> --- a/gcc/c-family/c-common.c
>>>> +++ b/gcc/c-family/c-common.c
>>>> @@ -511,6 +511,7 @@ const struct c_common_resword c_common_reswords[] =
>>>>    { "__underlying_type", RID_UNDERLYING_TYPE, D_CXXONLY },
>>>>    { "__volatile",    RID_VOLATILE,    0 },
>>>>    { "__volatile__",    RID_VOLATILE,    0 },
>>>> +  { "__GIMPLE",        RID_GIMPLE,    0 },
>>>
>>> I think we need a D_GIMPLE_ONLY flag which disables this reserved word
>>> when -fgimple is not
>>> in effect.  That's something to put on a list of TODOs once everything
>>> else works fine (it's not
>>> high priority but a requirement to merge to trunk).  For now you can
>>> at least put D_CONLY there
>>> (as -fgimple is a C only flag).
>>>
>>>>    { "alignas",        RID_ALIGNAS,    D_CXXONLY | D_CXX11 | D_CXXWARN },
>>>>    { "alignof",        RID_ALIGNOF,    D_CXXONLY | D_CXX11 | D_CXXWARN },
>>>>    { "asm",        RID_ASM,    D_ASM },
>>>> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
>>>> index 0295532..23a401d 100644
>>>> --- a/gcc/c-family/c-common.h
>>>> +++ b/gcc/c-family/c-common.h
>>>> @@ -104,6 +104,9 @@ enum rid
>>>>    RID_DFLOAT32, RID_DFLOAT64, RID_DFLOAT128,
>>>>    RID_FRACT, RID_ACCUM, RID_AUTO_TYPE, RID_BUILTIN_CALL_WITH_STATIC_CHAIN,
>>>>
>>>> +  /* "__GIMPLE", for the GIMPLE-parsing extension to the C frontend. */
>>>> +  RID_GIMPLE,
>>>> +
>>>>    /* C11 */
>>>>    RID_ALIGNAS, RID_GENERIC,
>>>>
>>>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
>>>> index 918df16..8ab56af 100644
>>>> --- a/gcc/c-family/c.opt
>>>> +++ b/gcc/c-family/c.opt
>>>> @@ -200,6 +200,10 @@ F
>>>>  Driver C ObjC C++ ObjC++ Joined Separate MissingArgError(missing path
>>>> after %qs)
>>>>  -F <dir>    Add <dir> to the end of the main framework include path.
>>>>
>>>> +fgimple
>>>> +C Var(flag_gimple) Init(0)
>>>> +Enable parsing GIMPLE
>>>> +
>>>>  H
>>>>  C ObjC C++ ObjC++
>>>>  Print the name of header files as they are used.
>>>> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
>>>> index 1cf4fb4..c5a4d3f 100644
>>>> --- a/gcc/c/c-parser.c
>>>> +++ b/gcc/c/c-parser.c
>>>> @@ -1396,6 +1396,7 @@ static bool c_parser_cilk_verify_simd (c_parser
>>>> *, enum pragma_context);
>>>>  static tree c_parser_array_notation (location_t, c_parser *, tree, tree);
>>>>  static tree c_parser_cilk_clause_vectorlength (c_parser *, tree, bool);
>>>>  static void c_parser_cilk_grainsize (c_parser *, bool *);
>>>> +static void c_parser_parse_gimple_body (c_parser *parser);
>>>>
>>>>  /* Parse a translation unit (C90 6.7, C99 6.9).
>>>>
>>>> @@ -1638,6 +1639,7 @@ c_parser_declaration_or_fndef (c_parser *parser,
>>>> bool fndef_ok,
>>>>    tree all_prefix_attrs;
>>>>    bool diagnosed_no_specs = false;
>>>>    location_t here = c_parser_peek_token (parser)->location;
>>>> +  bool gimple_body_p = false;
>>>>
>>>>    if (static_assert_ok
>>>>        && c_parser_next_token_is_keyword (parser, RID_STATIC_ASSERT))
>>>> @@ -1687,6 +1689,17 @@ c_parser_declaration_or_fndef (c_parser
>>>> *parser, bool fndef_ok,
>>>>        c_parser_skip_to_end_of_block_or_statement (parser);
>>>>        return;
>>>>      }
>>>> +
>>>> +  if (c_parser_next_token_is (parser, CPP_KEYWORD))
>>>> +    {
>>>> +      c_token *kw_token = c_parser_peek_token (parser);
>>>> +      if (kw_token->keyword == RID_GIMPLE)
>>>> +    {
>>>> +      gimple_body_p = true;
>>>> +      c_parser_consume_token (parser);
>>>> +    }
>>>> +    }
>>>> +
>>>>    finish_declspecs (specs);
>>>>    bool auto_type_p = specs->typespec_word == cts_auto_type;
>>>>    if (c_parser_next_token_is (parser, CPP_SEMICOLON))
>>>> @@ -2102,6 +2115,14 @@ c_parser_declaration_or_fndef (c_parser
>>>> *parser, bool fndef_ok,
>>>>                     oacc_routine_clauses, false, first, true);
>>>>        DECL_STRUCT_FUNCTION (current_function_decl)->function_start_locus
>>>>      = c_parser_peek_token (parser)->location;
>>>> +
>>>> +      if (gimple_body_p && flag_gimple)
>>>> +    {
>>>> +      c_parser_parse_gimple_body (parser);
>>>> +      finish_function ();
>>>> +      return;
>>>> +    }
>>>> +
>>>>        fnbody = c_parser_compound_statement (parser);
>>>>        if (flag_cilkplus && contains_array_notation_expr (fnbody))
>>>>      fnbody = expand_array_notation_exprs (fnbody);
>>>> @@ -2123,7 +2144,7 @@ c_parser_declaration_or_fndef (c_parser *parser,
>>>> bool fndef_ok,
>>>>        add_stmt (fnbody);
>>>>        finish_function ();
>>>>      }
>>>> -
>>>> +
>>>>        timevar_pop (tv);
>>>>        break;
>>>>      }
>>>
>>> Always avoid stay changes like this.
>>>
>>>> @@ -18055,4 +18076,23 @@ c_parser_array_notation (location_t loc,
>>>> c_parser *parser, tree initial_index,
>>>>    return value_tree;
>>>>  }
>>>>
>>>> +/* Parse the body of a function declaration marked with "__GIMPLE".  */
>>>> +
>>>> +void
>>>> +c_parser_parse_gimple_body (c_parser *parser)
>>>> +{
>>>> +  if (!c_parser_require (parser, CPP_OPEN_BRACE, "expected %<{%>"))
>>>> +    return;
>>>> +
>>>> +  location_t loc1 = c_parser_peek_token (parser)->location;
>>>> +  inform (loc1, "start of GIMPLE");
>>>> +
>>>> +  while (c_parser_next_token_is_not (parser, CPP_CLOSE_BRACE))
>>>> +    {
>>>> +    c_parser_consume_token (parser);
>>>> +    }
>>>> +
>>>> +  c_parser_consume_token (parser);
>>>> +}
>>>> +
>>>>  #include "gt-c-c-parser.h"
>>>>
>>>>
>>>>
>>>>
>>>> GIMPLE function body consists of GIMPLE statements. For parsing GIMPLE
>>>> function body, we need to populate gimple body with gimple statements
>>>> which will emit GIMPLE as it is.
>>>>
>>>> I would like to hear suggestions about how can I emit GIMPLE directly from FE.
>>>
>>> You should be able to use gimple_build_* functions declared in gimple.h, for
>>> example
>>>
>>>   stmt = gimple_build_assign (res, PLUS_EXPR, op0, op1);
>>>
>>> will build an assignment to res computing op0 + op1.  res, op0 and op1 can be
>>> most easily parsed using the appropriate C parsing functions for identifiers.
>>>
>>> You'll need to queue stmts you built in a gimple_seq and attach that to the
>>> function via gimple_set_body (fndecl, sequence).
>>>
>>> I suggest you try with a "simple" example like
>>>
>>> int a;
>>>
>>> __GIMPLE void foo (void)
>>> {
>>>   a = 1;
>>> }
>>>
>>> going forward with handling a = 1 + 2; and then, sth you'll need very quickly,
>>> handle local declarations like
>>>
>>> int a;
>>> __GIMPLE int foo (void)
>>> {
>>>   int b;
>>>   b = a;
>>>   b = b + 1;
>>>   a = b;
>>> }
>>>
>>> this is sth C code like int foo (void) { a = a + 1; } is gimplified to.
>>>
>>> Richard.
>>>
>>>>
>>>>
>>>> Thanks,
>>>> Prasad Ghangal

Attachment: patch.diff
Description: Text document