Re: out of bounds access in insn-automata.c
On 03/23/2016 10:25 AM, Bernd Schmidt wrote:
On 03/23/2016 07:32 AM, Aldy Hernandez wrote:
int
maximal_insn_latency (rtx insn)
{
int insn_code;
if (insn == 0)
insn_code = DFA__ADVANCE_CYCLE;
else
{
insn_code = dfa_insn_code (as_a (insn));
if (insn_code > DFA__ADVANCE_CYCLE)
return 0;
}
return internal_maximal_insn_latency (insn_code, insn);
}
In the case where insn==0, insn_code is set to the size of
default_latencies[] which will get accessed in the return.
Does insn==0 never happen?
I suspect it never happens in this function. I'd add a gcc_assert to
that effect and try a bootstrap/test. Hmm, it seems to be a sel-sched
only thing so a normal bootstrap would be meaningless, but from the
context it looks fairly clearly like insn is always nonnull.
Vlad. Bernd. Thanks for your input.
I've added the assert on the caller (maximal_insn_latency), because as
you mentioned, the helper function is used for other things in which
insn==0 can happen.
Now we generate:
int
maximal_insn_latency (rtx insn)
{
int insn_code;
gcc_assert (insn != 0); // --- Added code.
if (insn == 0)
insn_code = DFA__ADVANCE_CYCLE;
else
{
insn_code = dfa_insn_code (as_a (insn));
if (insn_code > DFA__ADVANCE_CYCLE)
return 0;
}
return internal_maximal_insn_latency (insn_code, insn);
}
It looks like this block of code is written by a helper function that is
really intended for other purposes than for maximal_insn_latency. Might
be worth changing to
int insn_code = dfa_insn_code (as_a (insn));
gcc_assert (insn_code <= DFA__ADVANCE_CYCLE);
dfa_insn_code_* and friends can return > DFA__ADVANCE_CYCLE so I can't
put that assert on the helper function.
While I was at it, I changed the helper function comment to reflect what
it has been generating. It was wrong.
First round of tests was ok, but test machine died. OK pending tests?
Aldy
+
+ * genautomata.c (output_maximal_insn_latency_func): Assert that
+ insn is non-null.
+ (output_internal_insn_code_evaluation): Fix comment to match
+ generated code.
diff --git a/gcc/genautomata.c b/gcc/genautomata.c
index e3a6c59..91abd2e 100644
--- a/gcc/genautomata.c
+++ b/gcc/genautomata.c
@@ -8113,14 +8113,14 @@ output_internal_trans_func (void)
/* Output code
- if (insn != 0)
+ if (insn == 0)
+insn_code = DFA__ADVANCE_CYCLE;
+ else
{
insn_code = dfa_insn_code (insn);
if (insn_code > DFA__ADVANCE_CYCLE)
return code;
}
- else
-insn_code = DFA__ADVANCE_CYCLE;
where insn denotes INSN_NAME, insn_code denotes INSN_CODE_NAME, and
code denotes CODE. */
@@ -8527,6 +8527,7 @@ output_maximal_insn_latency_func (void)
"maximal_insn_latency", INSN_PARAMETER_NAME);
fprintf (output_file, "{\n int %s;\n",
INTERNAL_INSN_CODE_NAME);
+ fprintf (output_file, " gcc_assert (%s != 0);\n", INSN_PARAMETER_NAME);
output_internal_insn_code_evaluation (INSN_PARAMETER_NAME,
INTERNAL_INSN_CODE_NAME, 0);
fprintf (output_file, " return %s (%s, %s);\n}\n\n",
Re: [gimplefe] [gsoc16] Gimple Front End Project
On Thu, Mar 24, 2016 at 12:27 AM, Prasad Ghangal
wrote:
> Hi!
>
> I have attached my gsoc proposal, please review it. Let me know if I
> have missed or misunderstood anything
Please re-word the Abstract, it is really weird to read - I suggest to drop
any notion of RTL or "back end" and somehow mention that unit testing
is why "we want make changes in GIMPLE". Also stopping compilation
isn't in the scope of the project (and while maybe a nice thing to have
to do less work it is not a requirement for unit testing).
Thus Project Goals are only unit testing (which includes what you call
start/stop compilation).
During the discussion in the last comments we agreed on re-using the
C frontend for decls and types only and implement a completely separate
parser for the function bodies so the frontend can also emit GIMPLE directly.
Thus I'd scrap 4.A)/B) and instead write about
4.
A) Define GIMPLE syntax
Provide GIMPLE dumps with enough information to parse GIMPLE and
change them according to agreed on GIMPLE syntax
B) Add __GIMPLE extension to the C FE and implement a GIMPLE parser
For the suggested timeline I think that it is important for the "get
basic stuff working"
part to have all pieces of the project prototyped, including the pass manager
support. Otherwise there is no way to fully test part of the implementation.
I'd say modifying the gimple dumps can be done last as you can always write
testcases manually.
I realize that the student application deadline is tomorrow.
Richard.
>
> Thanks and Regards,
> Prasad Ghangal
>
>
>
>
>
> On 22 March 2016 at 19:23, Richard Biener wrote:
>> On Tue, Mar 22, 2016 at 2:45 PM, Prathamesh Kulkarni
>> wrote:
>>> On 22 March 2016 at 16:26, Richard Biener
>>> wrote:
On Tue, Mar 22, 2016 at 12:08 AM, Prasad Ghangal
wrote:
> Hi!
>
> How exactly can we achieve start stop compilation on specific pass (ie
> run single pass on input)?
>
> eg. $cgimple -ftree-copyrename foo.c
>
> should produce optimization result of -ftree-copyrename pass on foo.c
> input
You need pass manager support and annotate each function with information
on what passes should be run (in which order even?). I think for the GSoC
project specifying a starting pass for each function via the source, like
__GIMPLE (tree-copyrename) void foo (void)
{
...
}
and hacking the pass manager to honor that is enough.
>>> Um would annotating each function with pass order work for ipa passes too ?
>>
>> There is no single point of execution for an IPA pass so no - you can
>> tag it with
>> one of the umbrella passes I guess. I suppose we'd need some magic "phase"
>> tags for this, like simply "IPA". You then need to enable/disable IPA passes
>> you want to run.
>>
>> Richard.
>>
>>> Thanks,
>>> Prathamesh
Richard.
>
>
> On 21 March 2016 at 09:05, Trevor Saunders wrote:
>> On Mon, Mar 21, 2016 at 04:43:35AM +0530, Prasad Ghangal wrote:
>>> Hi!
>>>
>>> Sorry for the late reply.
>>>
>>> I was observing gimple dumps and my initial findings are, to parse
>>> gimple, we have to add support for following components to C FE
>>>
>>> *basic blocks
>>
>> I'd think you can probably make these enough like C labels that you
>> don't need to do anything special in the C fe to parse these. Just
>> removing the < and > gets you pretty close is that it?
>>
>>> *gimple labels and goto
>>
>> Similar I think.
>>
>>> *gimple phi functions
>>> iftmp_0_1 = PHI (ftmp_0_3, iftmp_0_4)
>>
>> yesI think you need to add something here. I think you can do it as a
>> builtin type function that expects its arguments to be labels or names
>> of variables.
>>
>>> *gimple switch
>>> switch (a_1) , case 1: , case 2: >
>>
>> I'd think we could make this more C like too.
>>
>>> *gimple exception handling
>>
>> yeah, though note exceptions are lowered pretty quickly so supporting
>> them with the explicit exception syntax probably isn't particularly
>> important.
>>
>>> *openmp functions like
>>> main._omp_fn.0 (void * .omp_data_i)
>>
>> I'd think you'd want to change the duping of this some to make it easier
>> to tell from struct.some.member.
>>
>>> Please correct me if I am wrong. Also point out if I am missing anything
>>
>> I think you might need to do something about variable names?
>>
>> Trev
>>
>>>
>>>
>>>
>>>
>>> On 18 March 2016 at 14:53, Richard Biener
>>> wrote:
>>> > On Fri, Mar 18, 2016 at 6:55 AM, Prathamesh Kulkarni
>>> > wrote:
>>> >> On 15 March 2016 at 20:46, Richard Biener
>>> >> wrote:
>>> >>> On Mon, Mar 14, 2016 at 7:27 PM, Michael Matz wrote:
>>> Hi,
>>>
>>> On Thu, 10 Mar 2016, Rich
Re: out of bounds access in insn-automata.c
On 03/24/2016 11:17 AM, Aldy Hernandez wrote: On 03/23/2016 10:25 AM, Bernd Schmidt wrote: It looks like this block of code is written by a helper function that is really intended for other purposes than for maximal_insn_latency. Might be worth changing to int insn_code = dfa_insn_code (as_a (insn)); gcc_assert (insn_code <= DFA__ADVANCE_CYCLE); dfa_insn_code_* and friends can return > DFA__ADVANCE_CYCLE so I can't put that assert on the helper function. So don't use the helper function? Just emit the block above directly. Bernd
Re: out of bounds access in insn-automata.c
Hi, On Thu, 24 Mar 2016, Bernd Schmidt wrote: > On 03/24/2016 11:17 AM, Aldy Hernandez wrote: > > On 03/23/2016 10:25 AM, Bernd Schmidt wrote: > > > It looks like this block of code is written by a helper function that is > > > really intended for other purposes than for maximal_insn_latency. Might > > > be worth changing to > > > int insn_code = dfa_insn_code (as_a (insn)); > > > gcc_assert (insn_code <= DFA__ADVANCE_CYCLE); > > > > dfa_insn_code_* and friends can return > DFA__ADVANCE_CYCLE so I can't > > put that assert on the helper function. > > So don't use the helper function? Just emit the block above directly. Let me chime in :) The function under scrutiny, maximal_insn_latency, was added as part of selective scheduling merge; at the same time, output_default_latencies was factored out of output_internal_insn_latency_func, and the pair of new functions output_internal_maximal_insn_latency_func/output_maximal_insn_latency_func tried to mirror existing pair of output_internal_insn_latency_func/output_insn_latency_func. In particular, output_insn_latency_func also invokes output_internal_insn_code_evaluation (twice, for each argument). This means that generated 'insn_latency' can also call 'internal_insn_latency' with DFA__ADVANCE_CYCLE in arguments. However, 'internal_insn_latency' then has a specially emitted 'if' statement that checks if either of the arguments is ' >= DFA__ADVANCE_CYCLE', and returns 0 in that case. So ultimately pre-existing code was checking ' > DFA__ADVANCE_CYCLE' first and ' >= DFA_ADVANCE_CYCLE' second (for no good reason as far as I can see), and when the new '_maximal_' functions were introduced, the second check was not duplicated in the new copy. So as long we are not looking for hacking it up further, I'd like to clean up both functions at the same time. If calling the 'internal_' variants with DFA__ADVANCE_CYCLE is rare, extending 'default_insn_latencies' by 1 zero element corresponding to DFA__ADVANCE_CYCLE is a simple suitable fix. If either DFA__ADVANCE_CYCLE is not guaranteed to be rare, or extending the table in that style is undesired, I suggest creating a variant of 'output_internal_insn_code_evaluation' that performs a '>=' rather than '>' test in the first place, and use it in both output_insn_latency_func and output_maximal_insn_latency_func. If acknowledged, I volunteer to regstrap on x86_64 and submit that in stage1. Thoughts? Thanks. Alexander
Re: [gimplefe] [gsoc16] Gimple Front End Project
On Thu, 2016-03-24 at 14:31 +0100, Richard Biener wrote:
> On Thu, Mar 24, 2016 at 12:27 AM, Prasad Ghangal
> wrote:
> > Hi!
> >
> > I have attached my gsoc proposal, please review it. Let me know if
> > I
> > have missed or misunderstood anything
>
> Please re-word the Abstract, it is really weird to read - I suggest
> to drop
> any notion of RTL or "back end" and somehow mention that unit testing
> is why "we want make changes in GIMPLE". Also stopping compilation
> isn't in the scope of the project (and while maybe a nice thing to
> have
> to do less work it is not a requirement for unit testing).
>
> Thus Project Goals are only unit testing (which includes what you
> call
> start/stop compilation).
>
> During the discussion in the last comments we agreed on re-using the
> C frontend for decls and types only and implement a completely
> separate
> parser for the function bodies so the frontend can also emit GIMPLE
> directly.
>
> Thus I'd scrap 4.A)/B) and instead write about
>
> 4.
> A) Define GIMPLE syntax
> Provide GIMPLE dumps with enough information to parse GIMPLE and
> change them according to agreed on GIMPLE syntax
>
> B) Add __GIMPLE extension to the C FE and implement a GIMPLE
> parser
>
> For the suggested timeline I think that it is important for the "get
> basic stuff working"
> part to have all pieces of the project prototyped, including the pass
> manager
> support. Otherwise there is no way to fully test part of the
> implementation.
> I'd say modifying the gimple dumps can be done last as you can always
> write
> testcases manually.
>
> I realize that the student application deadline is tomorrow.
>
> Richard.
I like the overall idea, though I agree with Richard that the abstract
needs some rewording (as he writes above).
I think the key idea is to be able to "round-trip" any time in the
middle end where we're in the GIMPLE representation: save to a textual
format, and then to be able to load that format back in, and then run
from there, so that we can test the effect of specific passes on
specific fragments of GIMPLE.
I'd be willing to co-mentor you for this project with Richard.
However, I'm about to leave for the airport for a vacation (in a few hours), so
the timing is rather bad. Sorry. I will have only intermittent access to email
until April 5th.
I have various ideas in this space, and I want to help with improving GCC's
unit-testing. FWIW, I've started experimenting with an *RTL* frontend, also
for the purpose of unit-testing, in this case for the later parts of the
compiler (so that we can unit-test e.g. the register allocators etc). This
could make a good "sister" project to the gimple frontend. (There will be some
things in common, and many differences).
Dave
>
>
> >
> > Thanks and Regards,
> > Prasad Ghangal
> >
> >
> >
> >
> >
> > On 22 March 2016 at 19:23, Richard Biener <
> > richard.guent...@gmail.com> wrote:
> > > On Tue, Mar 22, 2016 at 2:45 PM, Prathamesh Kulkarni
> > > wrote:
> > > > On 22 March 2016 at 16:26, Richard Biener <
> > > > richard.guent...@gmail.com> wrote:
> > > > > On Tue, Mar 22, 2016 at 12:08 AM, Prasad Ghangal
> > > > > wrote:
> > > > > > Hi!
> > > > > >
> > > > > > How exactly can we achieve start stop compilation on
> > > > > > specific pass (ie
> > > > > > run single pass on input)?
> > > > > >
> > > > > > eg. $cgimple -ftree-copyrename foo.c
> > > > > >
> > > > > > should produce optimization result of -ftree-copyrename
> > > > > > pass on foo.c input
> > > > >
> > > > > You need pass manager support and annotate each function with
> > > > > information
> > > > > on what passes should be run (in which order even?). I think
> > > > > for the GSoC
> > > > > project specifying a starting pass for each function via the
> > > > > source, like
> > > > >
> > > > > __GIMPLE (tree-copyrename) void foo (void)
> > > > > {
> > > > > ...
> > > > > }
> > > > >
> > > > > and hacking the pass manager to honor that is enough.
> > > > Um would annotating each function with pass order work for ipa
> > > > passes too ?
> > >
> > > There is no single point of execution for an IPA pass so no - you
> > > can
> > > tag it with
> > > one of the umbrella passes I guess. I suppose we'd need some
> > > magic "phase"
> > > tags for this, like simply "IPA". You then need to
> > > enable/disable IPA passes
> > > you want to run.
> > >
> > > Richard.
> > >
> > > > Thanks,
> > > > Prathamesh
> > > > >
> > > > > Richard.
> > > > >
> > > > > >
> > > > > >
> > > > > > On 21 March 2016 at 09:05, Trevor Saunders <
> > > > > > tbsau...@tbsaunde.org> wrote:
> > > > > > > On Mon, Mar 21, 2016 at 04:43:35AM +0530, Prasad Ghangal
> > > > > > > wrote:
> > > > > > > > Hi!
> > > > > > > >
> > > > > > > > Sorry for the late reply.
> > > > > > > >
> > > > > > > > I was observing gimple dumps and my initial findings
> > > > > > > > are, to parse
> > > > > > > > gimple, we have to add support for following components
>
Re: [gimplefe] [gsoc16] Gimple Front End Project
On March 24, 2016 6:30:29 PM GMT+01:00, David Malcolm
wrote:
>On Thu, 2016-03-24 at 14:31 +0100, Richard Biener wrote:
>> On Thu, Mar 24, 2016 at 12:27 AM, Prasad Ghangal
>> wrote:
>> > Hi!
>> >
>> > I have attached my gsoc proposal, please review it. Let me know if
>> > I
>> > have missed or misunderstood anything
>>
>> Please re-word the Abstract, it is really weird to read - I suggest
>> to drop
>> any notion of RTL or "back end" and somehow mention that unit testing
>> is why "we want make changes in GIMPLE". Also stopping compilation
>> isn't in the scope of the project (and while maybe a nice thing to
>> have
>> to do less work it is not a requirement for unit testing).
>>
>> Thus Project Goals are only unit testing (which includes what you
>> call
>> start/stop compilation).
>>
>> During the discussion in the last comments we agreed on re-using the
>> C frontend for decls and types only and implement a completely
>> separate
>> parser for the function bodies so the frontend can also emit GIMPLE
>> directly.
>>
>> Thus I'd scrap 4.A)/B) and instead write about
>>
>> 4.
>> A) Define GIMPLE syntax
>> Provide GIMPLE dumps with enough information to parse GIMPLE and
>> change them according to agreed on GIMPLE syntax
>>
>> B) Add __GIMPLE extension to the C FE and implement a GIMPLE
>> parser
>>
>> For the suggested timeline I think that it is important for the "get
>> basic stuff working"
>> part to have all pieces of the project prototyped, including the pass
>> manager
>> support. Otherwise there is no way to fully test part of the
>> implementation.
>> I'd say modifying the gimple dumps can be done last as you can always
>> write
>> testcases manually.
>>
>> I realize that the student application deadline is tomorrow.
>>
>> Richard.
>
>I like the overall idea, though I agree with Richard that the abstract
>needs some rewording (as he writes above).
>
>I think the key idea is to be able to "round-trip" any time in the
>middle end where we're in the GIMPLE representation: save to a textual
>format, and then to be able to load that format back in, and then run
>from there, so that we can test the effect of specific passes on
>specific fragments of GIMPLE.
>
>I'd be willing to co-mentor you for this project with Richard.
>
>However, I'm about to leave for the airport for a vacation (in a few
>hours), so the timing is rather bad. Sorry. I will have only
>intermittent access to email until April 5th.
>
>I have various ideas in this space, and I want to help with improving
>GCC's unit-testing. FWIW, I've started experimenting with an *RTL*
>frontend, also for the purpose of unit-testing, in this case for the
>later parts of the compiler (so that we can unit-test e.g. the register
>allocators etc). This could make a good "sister" project to the gimple
>frontend. (There will be some things in common, and many differences).
I would expect many similarities but a __RTL keyword for The bodies. Smaller
scope might be possible if you dont need to go all the way to assembler
generation.
Pass manager changes will be similar, though interesting side-effects with
regard to any mandatory IL analysis we usually do not perform on RTL
Richard.
>Dave
>
>
>>
>>
>> >
>> > Thanks and Regards,
>> > Prasad Ghangal
>> >
>> >
>> >
>> >
>> >
>> > On 22 March 2016 at 19:23, Richard Biener <
>> > richard.guent...@gmail.com> wrote:
>> > > On Tue, Mar 22, 2016 at 2:45 PM, Prathamesh Kulkarni
>> > > wrote:
>> > > > On 22 March 2016 at 16:26, Richard Biener <
>> > > > richard.guent...@gmail.com> wrote:
>> > > > > On Tue, Mar 22, 2016 at 12:08 AM, Prasad Ghangal
>> > > > > wrote:
>> > > > > > Hi!
>> > > > > >
>> > > > > > How exactly can we achieve start stop compilation on
>> > > > > > specific pass (ie
>> > > > > > run single pass on input)?
>> > > > > >
>> > > > > > eg. $cgimple -ftree-copyrename foo.c
>> > > > > >
>> > > > > > should produce optimization result of -ftree-copyrename
>> > > > > > pass on foo.c input
>> > > > >
>> > > > > You need pass manager support and annotate each function with
>> > > > > information
>> > > > > on what passes should be run (in which order even?). I think
>> > > > > for the GSoC
>> > > > > project specifying a starting pass for each function via the
>> > > > > source, like
>> > > > >
>> > > > > __GIMPLE (tree-copyrename) void foo (void)
>> > > > > {
>> > > > > ...
>> > > > > }
>> > > > >
>> > > > > and hacking the pass manager to honor that is enough.
>> > > > Um would annotating each function with pass order work for ipa
>> > > > passes too ?
>> > >
>> > > There is no single point of execution for an IPA pass so no - you
>> > > can
>> > > tag it with
>> > > one of the umbrella passes I guess. I suppose we'd need some
>> > > magic "phase"
>> > > tags for this, like simply "IPA". You then need to
>> > > enable/disable IPA passes
>> > > you want to run.
>> > >
>> > > Richard.
>> > >
>> > > > Thanks,
>> > > > Prathamesh
>> > > > >
>>
Re: [gimplefe] [gsoc16] Gimple Front End Project
On 24 March 2016 at 19:01, Richard Biener wrote:
> On Thu, Mar 24, 2016 at 12:27 AM, Prasad Ghangal
> wrote:
>> Hi!
>>
>> I have attached my gsoc proposal, please review it. Let me know if I
>> have missed or misunderstood anything
>
> Please re-word the Abstract, it is really weird to read - I suggest to drop
> any notion of RTL or "back end" and somehow mention that unit testing
> is why "we want make changes in GIMPLE". Also stopping compilation
> isn't in the scope of the project (and while maybe a nice thing to have
> to do less work it is not a requirement for unit testing).
>
> Thus Project Goals are only unit testing (which includes what you call
> start/stop compilation).
>
> During the discussion in the last comments we agreed on re-using the
> C frontend for decls and types only and implement a completely separate
> parser for the function bodies so the frontend can also emit GIMPLE directly.
>
> Thus I'd scrap 4.A)/B) and instead write about
>
> 4.
> A) Define GIMPLE syntax
> Provide GIMPLE dumps with enough information to parse GIMPLE and
> change them according to agreed on GIMPLE syntax
Little confused here. Don't we need to change syntax first and then
develop grammar to parse it?
>
> B) Add __GIMPLE extension to the C FE and implement a GIMPLE parser
>
> For the suggested timeline I think that it is important for the "get
> basic stuff working"
> part to have all pieces of the project prototyped, including the pass manager
> support. Otherwise there is no way to fully test part of the implementation.
> I'd say modifying the gimple dumps can be done last as you can always write
> testcases manually.
>
> I realize that the student application deadline is tomorrow.
>
> Richard.
>
>
>
>>
>> Thanks and Regards,
>> Prasad Ghangal
>>
>>
>>
>>
>>
>> On 22 March 2016 at 19:23, Richard Biener wrote:
>>> On Tue, Mar 22, 2016 at 2:45 PM, Prathamesh Kulkarni
>>> wrote:
On 22 March 2016 at 16:26, Richard Biener
wrote:
> On Tue, Mar 22, 2016 at 12:08 AM, Prasad Ghangal
> wrote:
>> Hi!
>>
>> How exactly can we achieve start stop compilation on specific pass (ie
>> run single pass on input)?
>>
>> eg. $cgimple -ftree-copyrename foo.c
>>
>> should produce optimization result of -ftree-copyrename pass on foo.c
>> input
>
> You need pass manager support and annotate each function with information
> on what passes should be run (in which order even?). I think for the GSoC
> project specifying a starting pass for each function via the source, like
>
> __GIMPLE (tree-copyrename) void foo (void)
> {
> ...
> }
>
> and hacking the pass manager to honor that is enough.
Um would annotating each function with pass order work for ipa passes too ?
>>>
>>> There is no single point of execution for an IPA pass so no - you can
>>> tag it with
>>> one of the umbrella passes I guess. I suppose we'd need some magic "phase"
>>> tags for this, like simply "IPA". You then need to enable/disable IPA
>>> passes
>>> you want to run.
>>>
>>> Richard.
>>>
Thanks,
Prathamesh
>
> Richard.
>
>>
>>
>> On 21 March 2016 at 09:05, Trevor Saunders wrote:
>>> On Mon, Mar 21, 2016 at 04:43:35AM +0530, Prasad Ghangal wrote:
Hi!
Sorry for the late reply.
I was observing gimple dumps and my initial findings are, to parse
gimple, we have to add support for following components to C FE
*basic blocks
>>>
>>> I'd think you can probably make these enough like C labels that you
>>> don't need to do anything special in the C fe to parse these. Just
>>> removing the < and > gets you pretty close is that it?
>>>
*gimple labels and goto
>>>
>>> Similar I think.
>>>
*gimple phi functions
iftmp_0_1 = PHI (ftmp_0_3, iftmp_0_4)
>>>
>>> yesI think you need to add something here. I think you can do it as a
>>> builtin type function that expects its arguments to be labels or names
>>> of variables.
>>>
*gimple switch
switch (a_1) , case 1: , case 2: >
>>>
>>> I'd think we could make this more C like too.
>>>
*gimple exception handling
>>>
>>> yeah, though note exceptions are lowered pretty quickly so supporting
>>> them with the explicit exception syntax probably isn't particularly
>>> important.
>>>
*openmp functions like
main._omp_fn.0 (void * .omp_data_i)
>>>
>>> I'd think you'd want to change the duping of this some to make it easier
>>> to tell from struct.some.member.
>>>
Please correct me if I am wrong. Also point out if I am missing
anything
>>>
>>> I think you might need to do something about variable names?
>>>
>>> Trev
>>>
On 18 Mar
Cross-compiler --sysroot discrepancy with default library and include paths
(Please CC me as I am not subscribed) It seems that when building a cross-compiler with a default --target but no default --with-sysroot, the default -isystem =/usr/include path is not automatically included when using --sysroot but default -L =/lib and -L =/usr/lib are included. (Full example output at end) I did not realize that --sysroot would not add in -isystem =/usr/include. Clang does auto include -isystem =/usr/include with --sysroot. The use-case here is for cross-building FreeBSD's userland against a fully populated sysroot that has target headers and libraries. The cross-compiler is provided in a pre-built package and does not have a hard-coded default TARGET_SYSTEM_ROOT in it. It is required to pass -isystem =/usr/include to do what I'm expecting. This behavior of not including =/usr/include is not documented in --sysroot or otherwise as far as I can tell. Adding the library search paths in but not /usr/include seems odd. In gcc.c the default library paths (standard_startfile_prefix_1, standard_startfile_prefix_2) are added in when cross_compile == '0' (not the case here) or if there is a target_system_root set. The target_system_root comes from both TARGET_SYSTEM_ROOT or dynamically from --sysroot. The include paths are only hard-coded at build time in cppdefault.c and the "native paths" of /usr/include are not added using basically the same logic as in gcc.c but only relies on the build-time TARGET_SYSTEM_ROOT not being defined. Granted the logic in cppdefault.c is intended to only bring in /usr/include and not =/usr/include, I think that --sysroot should also default to adding in -isystem =/usr/include. Otherwise something should be documented here. Here's the cross-compiler being used: > # /usr/local/bin/powerpc64-portbld-freebsd11.0-g++ -v > Using built-in specs. > COLLECT_GCC=/usr/local/bin/powerpc64-portbld-freebsd11.0-g++ > COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc/powerpc64-portbld-freebsd11.0/5.3.0/lto-wrapper > Target: powerpc64-portbld-freebsd11.0 > Configured with: ./../gcc-5.3.0/configure > --target=powerpc64-portbld-freebsd11.0 --disable-nls --enable-languages=c,c++ > --without-headers --with-gmp=/usr/local --with-pkgversion='FreeBSD Ports > Collection for powerpc64' --with-system-zlib > --with-as=/usr/local/bin/powerpc64-freebsd-as > --with-ld=/usr/local/bin/powerpc64-freebsd-ld --prefix=/usr/local > --localstatedir=/var --mandir=/usr/local/man --infodir=/usr/local/info/ > --build=x86_64-portbld-freebsd11.0 > Thread model: posix > gcc version 5.3.0 (FreeBSD Ports Collection for powerpc64) Library search paths with and without sysroot: > # /usr/obj/sparc64.sparc64/root/git/freebsd/tmp/usr/bin/cc -print-search-dirs > install: /usr/obj/sparc64.sparc64/root/git/freebsd/tmp/usr/libexec/ > programs: > =/usr/obj/sparc64.sparc64/root/git/freebsd/tmp/usr/bin/:/usr/obj/sparc64.sparc64/root/git/freebsd/tmp/usr/bin/:/usr/obj/sparc64.sparc64/root/git/freebsd/tmp/usr/libexec/:/usr/obj/sparc64.sparc64/root/git/freebsd/tmp/usr/libexec/:/usr/obj/sparc64.sparc64/root/git/freebsd/tmp/usr/libexec/ > libraries: > =/usr/obj/sparc64.sparc64/root/git/freebsd/tmp/usr/lib/:/usr/obj/sparc64.sparc64/root/git/freebsd/tmp/usr/lib/ > # /usr/local/bin/powerpc64-portbld-freebsd11.0-g++ -print-search-dirs > --sysroot /usr/obj/powerpc.powerpc64/root/git/freebsd/tmp > install: /usr/local/lib/gcc/powerpc64-portbld-freebsd11.0/5.3.0/ > programs: > =/usr/local/libexec/gcc/powerpc64-portbld-freebsd11.0/5.3.0/:/usr/local/libexec/gcc/powerpc64-portbld-freebsd11.0/5.3.0/:/usr/local/libexec/gcc/powerpc64-portbld-freebsd11.0/:/usr/local/lib/gcc/powerpc64-portbld-freebsd11.0/5.3.0/:/usr/local/lib/gcc/powerpc64-portbld-freebsd11.0/:/usr/local/lib/gcc/powerpc64-portbld-freebsd11.0/5.3.0/../../../../powerpc64-portbld-freebsd11.0/bin/powerpc64-portbld-freebsd11.0/5.3.0/:/usr/local/lib/gcc/powerpc64-portbld-freebsd11.0/5.3.0/../../../../powerpc64-portbld-freebsd11.0/bin/ > libraries: > =/usr/local/lib/gcc/powerpc64-portbld-freebsd11.0/5.3.0/:/usr/local/lib/gcc/powerpc64-portbld-freebsd11.0/5.3.0/../../../../powerpc64-portbld-freebsd11.0/lib/powerpc64-portbld-freebsd11.0/5.3.0/:/usr/local/lib/gcc/powerpc64-portbld-freebsd11.0/5.3.0/../../../../powerpc64-portbld-freebsd11.0/lib/:/usr/obj/powerpc.powerpc64/root/git/freebsd/tmp/lib/powerpc64-portbld-freebsd11.0/5.3.0/:/usr/obj/powerpc.powerpc64/root/git/freebsd/tmp/lib/:/usr/obj/powerpc.powerpc64/root/git/freebsd/tmp/usr/lib/powerpc64-portbld-freebsd11.0/5.3.0/:/usr/obj/powerpc.powerpc64/root/git/freebsd/tmp/usr/lib/ Include path with --sysroot: > # echo '' | /usr/local/bin/powerpc64-portbld-freebsd11.0-g++ -v -x c - -o > /dev/null --sysroot /usr/obj/powerpc.powerpc64/root/git/freebsd/tmp > Using built-in specs. > COLLECT_GCC=/usr/local/bin/powerpc64-portbld-freebsd11.0-g++ > COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc/powerpc64-portbld-freebsd11.0/5.3.0/lto-wrapper > Target: powerpc64-portbld-freebsd11.0 > Configured with: ./../gcc-5.3.0/configure
Re: [gimplefe] [gsoc16] Gimple Front End Project
On 25 March 2016 at 01:15, Prasad Ghangal wrote:
> On 24 March 2016 at 19:01, Richard Biener wrote:
>> On Thu, Mar 24, 2016 at 12:27 AM, Prasad Ghangal
>> wrote:
>>> Hi!
>>>
>>> I have attached my gsoc proposal, please review it. Let me know if I
>>> have missed or misunderstood anything
>>
>> Please re-word the Abstract, it is really weird to read - I suggest to drop
>> any notion of RTL or "back end" and somehow mention that unit testing
>> is why "we want make changes in GIMPLE". Also stopping compilation
>> isn't in the scope of the project (and while maybe a nice thing to have
>> to do less work it is not a requirement for unit testing).
>>
>> Thus Project Goals are only unit testing (which includes what you call
>> start/stop compilation).
>>
>> During the discussion in the last comments we agreed on re-using the
>> C frontend for decls and types only and implement a completely separate
>> parser for the function bodies so the frontend can also emit GIMPLE directly.
>>
>> Thus I'd scrap 4.A)/B) and instead write about
>>
>> 4.
>> A) Define GIMPLE syntax
>> Provide GIMPLE dumps with enough information to parse GIMPLE and
>> change them according to agreed on GIMPLE syntax
>
> Little confused here. Don't we need to change syntax first and then
> develop grammar to parse it?
Well the purpose of grammar is to define the syntax.
We would need to first define a grammar for GIMPLE and then implement
a (predictive) parser
that parses GIMPLE input according to our grammar and emits GIMPLE IR.
contrived eg:
stmt_list: stmt stmt_list_1
stmt_list_1: stmt stmt_list_1 | epsilon
stmt: lvalue_operand '=' assign_stmt
| 'if' cond_stmt
assign_stmt: operand | operator operand | operand operator operand
operand: INT | lvalue_operand
lvalue_operand: ID
operator: '+'
As pointed out by Richard, dumps could be modified later to adjust to
GIMPLE syntax,
initially you could write test-cases by hand.
Thanks,
Prathamesh
>
>>
>> B) Add __GIMPLE extension to the C FE and implement a GIMPLE parser
>>
>> For the suggested timeline I think that it is important for the "get
>> basic stuff working"
>> part to have all pieces of the project prototyped, including the pass manager
>> support. Otherwise there is no way to fully test part of the implementation.
>> I'd say modifying the gimple dumps can be done last as you can always write
>> testcases manually.
>>
>> I realize that the student application deadline is tomorrow.
>>
>> Richard.
>>
>>
>>
>>>
>>> Thanks and Regards,
>>> Prasad Ghangal
>>>
>>>
>>>
>>>
>>>
>>> On 22 March 2016 at 19:23, Richard Biener
>>> wrote:
On Tue, Mar 22, 2016 at 2:45 PM, Prathamesh Kulkarni
wrote:
> On 22 March 2016 at 16:26, Richard Biener
> wrote:
>> On Tue, Mar 22, 2016 at 12:08 AM, Prasad Ghangal
>> wrote:
>>> Hi!
>>>
>>> How exactly can we achieve start stop compilation on specific pass (ie
>>> run single pass on input)?
>>>
>>> eg. $cgimple -ftree-copyrename foo.c
>>>
>>> should produce optimization result of -ftree-copyrename pass on foo.c
>>> input
>>
>> You need pass manager support and annotate each function with information
>> on what passes should be run (in which order even?). I think for the
>> GSoC
>> project specifying a starting pass for each function via the source, like
>>
>> __GIMPLE (tree-copyrename) void foo (void)
>> {
>> ...
>> }
>>
>> and hacking the pass manager to honor that is enough.
> Um would annotating each function with pass order work for ipa passes too
> ?
There is no single point of execution for an IPA pass so no - you can
tag it with
one of the umbrella passes I guess. I suppose we'd need some magic "phase"
tags for this, like simply "IPA". You then need to enable/disable IPA
passes
you want to run.
Richard.
> Thanks,
> Prathamesh
>>
>> Richard.
>>
>>>
>>>
>>> On 21 March 2016 at 09:05, Trevor Saunders
>>> wrote:
On Mon, Mar 21, 2016 at 04:43:35AM +0530, Prasad Ghangal wrote:
> Hi!
>
> Sorry for the late reply.
>
> I was observing gimple dumps and my initial findings are, to parse
> gimple, we have to add support for following components to C FE
>
> *basic blocks
I'd think you can probably make these enough like C labels that you
don't need to do anything special in the C fe to parse these. Just
removing the < and > gets you pretty close is that it?
> *gimple labels and goto
Similar I think.
> *gimple phi functions
> iftmp_0_1 = PHI (ftmp_0_3, iftmp_0_4)
yesI think you need to add something here. I think you can do it as a
builtin type function that expects its arguments to be labels or names
of variables.
>>
Re: out of bounds access in insn-automata.c
On 03/24/2016 10:02 AM, Alexander Monakov wrote: Hi, On Thu, 24 Mar 2016, Bernd Schmidt wrote: On 03/24/2016 11:17 AM, Aldy Hernandez wrote: On 03/23/2016 10:25 AM, Bernd Schmidt wrote: It looks like this block of code is written by a helper function that is really intended for other purposes than for maximal_insn_latency. Might be worth changing to int insn_code = dfa_insn_code (as_a (insn)); gcc_assert (insn_code <= DFA__ADVANCE_CYCLE); dfa_insn_code_* and friends can return > DFA__ADVANCE_CYCLE so I can't put that assert on the helper function. So don't use the helper function? Just emit the block above directly. Let me chime in :) The function under scrutiny, maximal_insn_latency, was added as part of selective scheduling merge; at the same time, output_default_latencies was factored out of output_internal_insn_latency_func, and the pair of new functions output_internal_maximal_insn_latency_func/output_maximal_insn_latency_func tried to mirror existing pair of output_internal_insn_latency_func/output_insn_latency_func. In particular, output_insn_latency_func also invokes output_internal_insn_code_evaluation (twice, for each argument). This means that generated 'insn_latency' can also call 'internal_insn_latency' with DFA__ADVANCE_CYCLE in arguments. However, 'internal_insn_latency' then has a specially emitted 'if' statement that checks if either of the arguments is ' >= DFA__ADVANCE_CYCLE', and returns 0 in that case. So ultimately pre-existing code was checking ' > DFA__ADVANCE_CYCLE' first and ' >= DFA_ADVANCE_CYCLE' second (for no good reason as far as I can see), and when the new '_maximal_' functions were introduced, the second check was not duplicated in the new copy. So as long we are not looking for hacking it up further, I'd like to clean up both functions at the same time. If calling the 'internal_' variants with DFA__ADVANCE_CYCLE is rare, extending 'default_insn_latencies' by 1 zero element corresponding to DFA__ADVANCE_CYCLE is a simple suitable fix. If either DFA__ADVANCE_CYCLE is not guaranteed to be rare, or extending the table in that style is undesired, I suggest creating a variant of 'output_internal_insn_code_evaluation' that performs a '>=' rather than '>' test in the first place, and use it in both output_insn_latency_func and output_maximal_insn_latency_func. If acknowledged, I volunteer to regstrap on x86_64 and submit that in stage1. Thoughts? If Bernd is fine with this, I'm happy to retract my patch and any possible followups. I'm just interested in having no path causing a possible out of bounds access. If your patch will do that, I'm cool. Aldy
