On Wed, Jun 8, 2016 at 10:57 PM, Prathamesh Kulkarni
<prathamesh.kulka...@linaro.org> wrote:
> On 6 June 2016 at 15:49, Richard Biener <richard.guent...@gmail.com> wrote:
>> On Mon, Jun 6, 2016 at 11:27 AM, Prasad Ghangal
>> <prasad.ghan...@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().
I think for all of the above we should somewhat refactor the current
code - I can help with this and for the scope of the GSoC project
"hacks" like the above ones are ok. If you need to add such please
make sure to add FIXME comments before them and explain why
they are there.
Before starting any refactoring I'd like to go forward with hacks as
needed to get a clearer direction as to what refactorings are needed.
> 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 ?
Yes, PHIs will make things interesting. Given that the GIMPLE FE
is supposed to be usable as unit-testing input we want to be able to
preserve the CFG anyway.
That said, when we get to control flow and PHIs we should decide in what
kind of state we want to emit the GIMPLE IL. The simplest thing would
be (for now?) to always have a CFG built by the frontend and thus basically
emit new functions as "lowered" and thus skil all_lowering_passes. I'd
say for the scope of the GSoC project that is the best thing to do.
Thanks,
Richard.
> Thanks,
> Prathamesh
>>
>> Richard.
>>
>>>
>>>
>>> Thanks,
>>> Prasad Ghangal
>>>
>>> On 31 May 2016 at 15:57, Richard Biener <richard.guent...@gmail.com> wrote:
>>>> On Mon, May 30, 2016 at 5:15 PM, Prasad Ghangal
>>>> <prasad.ghan...@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
