Re: [gimplefe] Parsing __GIMPLE function body
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 ?
Thanks,
Prasad Ghangal
On 31 May 2016 at 15:57, Richard Biener wrote:
> On Mon, May 30, 2016 at 5:15 PM, Prasad Ghangal
> 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 Add 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_pa
Re: [gimplefe] Parsing __GIMPLE function body
On Mon, Jun 6, 2016 at 11:27 AM, Prasad Ghangal
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 0x01798f21 in lower_function_body ()
at /space/rguenther/src/svn/GSoC/gcc/gimple-low.c:93
93gcc_assert (gimple_seq_first (body) == gimple_seq_last (body)
(gdb) l
88gimple *bind;
89gimple *x;
90
91/* The gimplifier should've left a body of exactly one statement,
92 namely a GIMPLE_BIND. */
93gcc_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 ;)
Richard.
>
>
> Thanks,
> Prasad Ghangal
>
> On 31 May 2016 at 15:57, Richard Biener wrote:
>> On Mon, May 30, 2016 at 5:15 PM, Prasad Ghangal
>> 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 Add 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 u
Re: Implementing atomic load as compare-and-swap for read-only memory
On 06/03/2016 02:26 PM, Torvald Riegel wrote: On Fri, 2016-06-03 at 12:03 +0200, Jakub Jelinek wrote: I guess it is a tough decision. If you don't have HW instruction to read say double word aligned integer atomically, if you don't implement atomic load on it through compare and swap (which indeed won't work with read-only memory), then you probably need to fall back to locks in libatomic. And that would be fine, IMO. If you can't even load atomically, doing something useful with this type will be hard except in special cases. It's possible to do this (in an ABI-compatible fashion even) with kernel support. I doubt it makes sense to try to fix this with lock fallback (which breaks in so many unexpected ways). Also, doing a CAS (compare-and-swap) and thus potentially bringing in the cache line in exclusive mode can be a lot more costly than what users might expect from a load. A short critical section might not be much slower. The performance problem is difficult to address, of course. Kernel emulation likely isn't cheap at all. Florian
