Re: [PATCH, Pointer Bounds Checker 13/x] Early versioning
On 05/29/14 05:05, Ilya Enkovich wrote: Hi, This patch allows to perform function versioning when some structures are not available yet. It is required to make clones for Pointer Bounds Checker right after SSA build. Bootstrapped and tested on linux-x86_64. Thanks, Ilya -- gcc/ 2014-05-29 Ilya Enkovich * tree-inline.c (copy_cfg_body): Check loop tree existence before accessing it. (tree_function_versioning): Check DF info existence before accessing it. diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c index 4293241..23fef90 100644 --- a/gcc/tree-inline.c +++ b/gcc/tree-inline.c @@ -2544,7 +2544,8 @@ copy_cfg_body (copy_body_data * id, gcov_type count, int frequency_scale, /* If the loop tree in the source function needed fixup, mark the destination loop tree for fixup, too. */ - if (loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP) + if (loops_for_fn (src_cfun) + && loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP) loops_state_set (LOOPS_NEED_FIXUP); Hmm, so if I understand things correctly, src_fun has no loop structures attached, thus there's nothing to copy. Presumably at some later point we build loop structures for the copy from scratch? Similarly for the PTA info, we just build it from scratch in the copy at some point? Assuming the answers to both are yes, then this patch is OK for the trunk when the rest of the patches are approved. It's not great, bit it's OK. jeff
Re: [PATCH, Pointer Bounds Checker 13/x] Early versioning
On 30 May 10:59, Jeff Law wrote:
> On 05/29/14 05:05, Ilya Enkovich wrote:
> >Hi,
> >
> >This patch allows to perform function versioning when some structures are
> >not available yet. It is required to make clones for Pointer Bounds Checker
> >right after SSA build.
> >
> >Bootstrapped and tested on linux-x86_64.
> >
> >Thanks,
> >Ilya
> >--
> >gcc/
> >
> >2014-05-29 Ilya Enkovich
> >
> > * tree-inline.c (copy_cfg_body): Check loop tree
> > existence before accessing it.
> > (tree_function_versioning): Check DF info existence
> > before accessing it.
> >
> >diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
> >index 4293241..23fef90 100644
> >--- a/gcc/tree-inline.c
> >+++ b/gcc/tree-inline.c
> >@@ -2544,7 +2544,8 @@ copy_cfg_body (copy_body_data * id, gcov_type count,
> >int frequency_scale,
> >
> >/* If the loop tree in the source function needed fixup, mark the
> > destination loop tree for fixup, too. */
> >- if (loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
> >+ if (loops_for_fn (src_cfun)
> >+ && loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
> > loops_state_set (LOOPS_NEED_FIXUP);
> Hmm, so if I understand things correctly, src_fun has no loop
> structures attached, thus there's nothing to copy. Presumably at
> some later point we build loop structures for the copy from scratch?
I suppose it is just a simple bug with absent NULL pointer check. Here is
original code:
/* Duplicate the loop tree, if available and wanted. */
if (loops_for_fn (src_cfun) != NULL
&& current_loops != NULL)
{
copy_loops (id, entry_block_map->loop_father,
get_loop (src_cfun, 0));
/* Defer to cfgcleanup to update loop-father fields of basic-blocks. */
loops_state_set (LOOPS_NEED_FIXUP);
}
/* If the loop tree in the source function needed fixup, mark the
destination loop tree for fixup, too. */
if (loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
loops_state_set (LOOPS_NEED_FIXUP);
As you may see we have check for absent loops structure in the first
if-statement and no check in the second one. I hit segfault and added the
check.
>
> Similarly for the PTA info, we just build it from scratch in the
> copy at some point?
Here we also have conditional access like
/* Reset the escaped solution. */
if (cfun->gimple_df)
pt_solution_reset (&cfun->gimple_df->escaped);
and following unconditional I've fixed.
>
> Assuming the answers to both are yes, then this patch is OK for the
> trunk when the rest of the patches are approved. It's not great,
> bit it's OK.
Thanks!
Ilya
>
> jeff
>
Re: [PATCH, Pointer Bounds Checker 13/x] Early versioning
On Mon, Jun 2, 2014 at 12:48 PM, Ilya Enkovich wrote:
> On 30 May 10:59, Jeff Law wrote:
>> On 05/29/14 05:05, Ilya Enkovich wrote:
>> >Hi,
>> >
>> >This patch allows to perform function versioning when some structures are
>> >not available yet. It is required to make clones for Pointer Bounds
>> >Checker right after SSA build.
>> >
>> >Bootstrapped and tested on linux-x86_64.
>> >
>> >Thanks,
>> >Ilya
>> >--
>> >gcc/
>> >
>> >2014-05-29 Ilya Enkovich
>> >
>> > * tree-inline.c (copy_cfg_body): Check loop tree
>> > existence before accessing it.
>> > (tree_function_versioning): Check DF info existence
>> > before accessing it.
>> >
>> >diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
>> >index 4293241..23fef90 100644
>> >--- a/gcc/tree-inline.c
>> >+++ b/gcc/tree-inline.c
>> >@@ -2544,7 +2544,8 @@ copy_cfg_body (copy_body_data * id, gcov_type count,
>> >int frequency_scale,
>> >
>> >/* If the loop tree in the source function needed fixup, mark the
>> > destination loop tree for fixup, too. */
>> >- if (loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
>> >+ if (loops_for_fn (src_cfun)
>> >+ && loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
>> > loops_state_set (LOOPS_NEED_FIXUP);
>> Hmm, so if I understand things correctly, src_fun has no loop
>> structures attached, thus there's nothing to copy. Presumably at
>> some later point we build loop structures for the copy from scratch?
> I suppose it is just a simple bug with absent NULL pointer check. Here is
> original code:
>
> /* Duplicate the loop tree, if available and wanted. */
> if (loops_for_fn (src_cfun) != NULL
> && current_loops != NULL)
> {
> copy_loops (id, entry_block_map->loop_father,
> get_loop (src_cfun, 0));
> /* Defer to cfgcleanup to update loop-father fields of basic-blocks. */
> loops_state_set (LOOPS_NEED_FIXUP);
> }
>
> /* If the loop tree in the source function needed fixup, mark the
> destination loop tree for fixup, too. */
> if (loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
> loops_state_set (LOOPS_NEED_FIXUP);
>
> As you may see we have check for absent loops structure in the first
> if-statement and no check in the second one. I hit segfault and added the
> check.
Actually after SSA is built we always have loops (loops are built once
we build the CFG which happens earlier). So all the above checks
are no longer necessary now.
>>
>> Similarly for the PTA info, we just build it from scratch in the
>> copy at some point?
>
> Here we also have conditional access like
>
> /* Reset the escaped solution. */
> if (cfun->gimple_df)
> pt_solution_reset (&cfun->gimple_df->escaped);
>
> and following unconditional I've fixed.
Likewise. The init_data_structures pass initializes this (init_tree_ssa).
So I'm not sure why you need all this given we are in SSA form?
Richard.
>>
>> Assuming the answers to both are yes, then this patch is OK for the
>> trunk when the rest of the patches are approved. It's not great,
>> bit it's OK.
>
> Thanks!
> Ilya
>
>>
>> jeff
>>
Re: [PATCH, Pointer Bounds Checker 13/x] Early versioning
2014-06-02 15:56 GMT+04:00 Richard Biener :
> On Mon, Jun 2, 2014 at 12:48 PM, Ilya Enkovich wrote:
>> On 30 May 10:59, Jeff Law wrote:
>>> On 05/29/14 05:05, Ilya Enkovich wrote:
>>> >Hi,
>>> >
>>> >This patch allows to perform function versioning when some structures are
>>> >not available yet. It is required to make clones for Pointer Bounds
>>> >Checker right after SSA build.
>>> >
>>> >Bootstrapped and tested on linux-x86_64.
>>> >
>>> >Thanks,
>>> >Ilya
>>> >--
>>> >gcc/
>>> >
>>> >2014-05-29 Ilya Enkovich
>>> >
>>> > * tree-inline.c (copy_cfg_body): Check loop tree
>>> > existence before accessing it.
>>> > (tree_function_versioning): Check DF info existence
>>> > before accessing it.
>>> >
>>> >diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
>>> >index 4293241..23fef90 100644
>>> >--- a/gcc/tree-inline.c
>>> >+++ b/gcc/tree-inline.c
>>> >@@ -2544,7 +2544,8 @@ copy_cfg_body (copy_body_data * id, gcov_type count,
>>> >int frequency_scale,
>>> >
>>> >/* If the loop tree in the source function needed fixup, mark the
>>> > destination loop tree for fixup, too. */
>>> >- if (loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
>>> >+ if (loops_for_fn (src_cfun)
>>> >+ && loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
>>> > loops_state_set (LOOPS_NEED_FIXUP);
>>> Hmm, so if I understand things correctly, src_fun has no loop
>>> structures attached, thus there's nothing to copy. Presumably at
>>> some later point we build loop structures for the copy from scratch?
>> I suppose it is just a simple bug with absent NULL pointer check. Here is
>> original code:
>>
>> /* Duplicate the loop tree, if available and wanted. */
>> if (loops_for_fn (src_cfun) != NULL
>> && current_loops != NULL)
>> {
>> copy_loops (id, entry_block_map->loop_father,
>> get_loop (src_cfun, 0));
>> /* Defer to cfgcleanup to update loop-father fields of basic-blocks.
>> */
>> loops_state_set (LOOPS_NEED_FIXUP);
>> }
>>
>> /* If the loop tree in the source function needed fixup, mark the
>> destination loop tree for fixup, too. */
>> if (loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
>> loops_state_set (LOOPS_NEED_FIXUP);
>>
>> As you may see we have check for absent loops structure in the first
>> if-statement and no check in the second one. I hit segfault and added the
>> check.
>
> Actually after SSA is built we always have loops (loops are built once
> we build the CFG which happens earlier). So all the above checks
> are no longer necessary now.
>
>>>
>>> Similarly for the PTA info, we just build it from scratch in the
>>> copy at some point?
>>
>> Here we also have conditional access like
>>
>> /* Reset the escaped solution. */
>> if (cfun->gimple_df)
>> pt_solution_reset (&cfun->gimple_df->escaped);
>>
>> and following unconditional I've fixed.
>
> Likewise. The init_data_structures pass initializes this (init_tree_ssa).
>
> So I'm not sure why you need all this given we are in SSA form?
Instrumentation clones are created before we are in SSA form. I do it
before early local passes.
Ilya
>
> Richard.
>
>>>
>>> Assuming the answers to both are yes, then this patch is OK for the
>>> trunk when the rest of the patches are approved. It's not great,
>>> bit it's OK.
>>
>> Thanks!
>> Ilya
>>
>>>
>>> jeff
>>>
Re: [PATCH, Pointer Bounds Checker 13/x] Early versioning
On 06/02/14 04:48, Ilya Enkovich wrote:
Hmm, so if I understand things correctly, src_fun has no loop
structures attached, thus there's nothing to copy. Presumably at
some later point we build loop structures for the copy from scratch?
I suppose it is just a simple bug with absent NULL pointer check. Here is
original code:
/* Duplicate the loop tree, if available and wanted. */
if (loops_for_fn (src_cfun) != NULL
&& current_loops != NULL)
{
copy_loops (id, entry_block_map->loop_father,
get_loop (src_cfun, 0));
/* Defer to cfgcleanup to update loop-father fields of basic-blocks. */
loops_state_set (LOOPS_NEED_FIXUP);
}
/* If the loop tree in the source function needed fixup, mark the
destination loop tree for fixup, too. */
if (loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
loops_state_set (LOOPS_NEED_FIXUP);
As you may see we have check for absent loops structure in the first
if-statement and no check in the second one. I hit segfault and added the
check.
Downthread you indicated you're not in SSA form which might explain the
inconsistency here. If so, then we need to make sure that the loop & df
structures do get set up properly later.
Jeff
Re: [PATCH, Pointer Bounds Checker 13/x] Early versioning
2014-06-02 21:27 GMT+04:00 Jeff Law :
> On 06/02/14 04:48, Ilya Enkovich wrote:
>>>
>>> Hmm, so if I understand things correctly, src_fun has no loop
>>> structures attached, thus there's nothing to copy. Presumably at
>>> some later point we build loop structures for the copy from scratch?
>>
>> I suppose it is just a simple bug with absent NULL pointer check. Here is
>> original code:
>>
>>/* Duplicate the loop tree, if available and wanted. */
>>if (loops_for_fn (src_cfun) != NULL
>>&& current_loops != NULL)
>> {
>>copy_loops (id, entry_block_map->loop_father,
>>get_loop (src_cfun, 0));
>>/* Defer to cfgcleanup to update loop-father fields of
>> basic-blocks. */
>>loops_state_set (LOOPS_NEED_FIXUP);
>> }
>>
>>/* If the loop tree in the source function needed fixup, mark the
>> destination loop tree for fixup, too. */
>>if (loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
>> loops_state_set (LOOPS_NEED_FIXUP);
>>
>> As you may see we have check for absent loops structure in the first
>> if-statement and no check in the second one. I hit segfault and added the
>> check.
>
> Downthread you indicated you're not in SSA form which might explain the
> inconsistency here. If so, then we need to make sure that the loop & df
> structures do get set up properly later.
That is what init_data_structures pass will do for us as Richard pointed. Right?
Ilya
>
> Jeff
Re: [PATCH, Pointer Bounds Checker 13/x] Early versioning
On Tue, Jun 3, 2014 at 7:55 AM, Ilya Enkovich wrote:
> 2014-06-02 21:27 GMT+04:00 Jeff Law :
>> On 06/02/14 04:48, Ilya Enkovich wrote:
Hmm, so if I understand things correctly, src_fun has no loop
structures attached, thus there's nothing to copy. Presumably at
some later point we build loop structures for the copy from scratch?
>>>
>>> I suppose it is just a simple bug with absent NULL pointer check. Here is
>>> original code:
>>>
>>>/* Duplicate the loop tree, if available and wanted. */
>>>if (loops_for_fn (src_cfun) != NULL
>>>&& current_loops != NULL)
>>> {
>>>copy_loops (id, entry_block_map->loop_father,
>>>get_loop (src_cfun, 0));
>>>/* Defer to cfgcleanup to update loop-father fields of
>>> basic-blocks. */
>>>loops_state_set (LOOPS_NEED_FIXUP);
>>> }
>>>
>>>/* If the loop tree in the source function needed fixup, mark the
>>> destination loop tree for fixup, too. */
>>>if (loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
>>> loops_state_set (LOOPS_NEED_FIXUP);
>>>
>>> As you may see we have check for absent loops structure in the first
>>> if-statement and no check in the second one. I hit segfault and added the
>>> check.
>>
>> Downthread you indicated you're not in SSA form which might explain the
>> inconsistency here. If so, then we need to make sure that the loop & df
>> structures do get set up properly later.
>
> That is what init_data_structures pass will do for us as Richard pointed.
> Right?
loops are set up during the CFG construction and thus are available everywhere.
the df structures are set up in init_data_structures pass which is run before
going into SSA form (I'd like to somehow cleanup that area).
Richard.
> Ilya
>
>>
>> Jeff
Re: [PATCH, Pointer Bounds Checker 13/x] Early versioning
On 06/03/14 03:29, Richard Biener wrote:
On Tue, Jun 3, 2014 at 7:55 AM, Ilya Enkovich wrote:
2014-06-02 21:27 GMT+04:00 Jeff Law :
On 06/02/14 04:48, Ilya Enkovich wrote:
Hmm, so if I understand things correctly, src_fun has no loop
structures attached, thus there's nothing to copy. Presumably at
some later point we build loop structures for the copy from scratch?
I suppose it is just a simple bug with absent NULL pointer check. Here is
original code:
/* Duplicate the loop tree, if available and wanted. */
if (loops_for_fn (src_cfun) != NULL
&& current_loops != NULL)
{
copy_loops (id, entry_block_map->loop_father,
get_loop (src_cfun, 0));
/* Defer to cfgcleanup to update loop-father fields of
basic-blocks. */
loops_state_set (LOOPS_NEED_FIXUP);
}
/* If the loop tree in the source function needed fixup, mark the
destination loop tree for fixup, too. */
if (loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
loops_state_set (LOOPS_NEED_FIXUP);
As you may see we have check for absent loops structure in the first
if-statement and no check in the second one. I hit segfault and added the
check.
Downthread you indicated you're not in SSA form which might explain the
inconsistency here. If so, then we need to make sure that the loop & df
structures do get set up properly later.
That is what init_data_structures pass will do for us as Richard pointed. Right?
loops are set up during the CFG construction and thus are available everywhere.
Which would argue that the hunk that checks for the loop tree's
existence before accessing it should not be needed. Ilya -- is it
possible you hit this prior to Richi's work to build the loop structures
as part of CFG construction and maintain them throughout compilation.
the df structures are set up in init_data_structures pass which is run before
going into SSA form (I'd like to somehow cleanup that area).
OK. So this part should be approved since we've established this code
is running prior to going into SSA form.
jeff
Re: [PATCH, Pointer Bounds Checker 13/x] Early versioning
2014-06-04 10:46 GMT+04:00 Jeff Law :
> On 06/03/14 03:29, Richard Biener wrote:
>>
>> On Tue, Jun 3, 2014 at 7:55 AM, Ilya Enkovich
>> wrote:
>>>
>>> 2014-06-02 21:27 GMT+04:00 Jeff Law :
On 06/02/14 04:48, Ilya Enkovich wrote:
>>
>>
>> Hmm, so if I understand things correctly, src_fun has no loop
>> structures attached, thus there's nothing to copy. Presumably at
>> some later point we build loop structures for the copy from scratch?
>
>
> I suppose it is just a simple bug with absent NULL pointer check. Here
> is
> original code:
>
> /* Duplicate the loop tree, if available and wanted. */
> if (loops_for_fn (src_cfun) != NULL
> && current_loops != NULL)
> {
> copy_loops (id, entry_block_map->loop_father,
> get_loop (src_cfun, 0));
> /* Defer to cfgcleanup to update loop-father fields of
> basic-blocks. */
> loops_state_set (LOOPS_NEED_FIXUP);
> }
>
> /* If the loop tree in the source function needed fixup, mark the
>destination loop tree for fixup, too. */
> if (loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
> loops_state_set (LOOPS_NEED_FIXUP);
>
> As you may see we have check for absent loops structure in the first
> if-statement and no check in the second one. I hit segfault and added
> the
> check.
Downthread you indicated you're not in SSA form which might explain the
inconsistency here. If so, then we need to make sure that the loop & df
structures do get set up properly later.
>>>
>>>
>>> That is what init_data_structures pass will do for us as Richard pointed.
>>> Right?
>>
>>
>> loops are set up during the CFG construction and thus are available
>> everywhere.
>
> Which would argue that the hunk that checks for the loop tree's existence
> before accessing it should not be needed. Ilya -- is it possible you hit
> this prior to Richi's work to build the loop structures as part of CFG
> construction and maintain them throughout compilation.
CFG build pass is in lowering stage and happens before versioning.
Probably I just hit some bug and hide it with my check. I'll try to
remove this check and reproduce a segfault to see why it happens.
Ilya
>
>
>
>> the df structures are set up in init_data_structures pass which is run
>> before
>> going into SSA form (I'd like to somehow cleanup that area).
>
> OK. So this part should be approved since we've established this code is
> running prior to going into SSA form.
>
> jeff
>
Re: [PATCH, Pointer Bounds Checker 13/x] Early versioning
On Wed, Jun 4, 2014 at 8:46 AM, Jeff Law wrote:
> On 06/03/14 03:29, Richard Biener wrote:
>>
>> On Tue, Jun 3, 2014 at 7:55 AM, Ilya Enkovich
>> wrote:
>>>
>>> 2014-06-02 21:27 GMT+04:00 Jeff Law :
On 06/02/14 04:48, Ilya Enkovich wrote:
>>
>>
>> Hmm, so if I understand things correctly, src_fun has no loop
>> structures attached, thus there's nothing to copy. Presumably at
>> some later point we build loop structures for the copy from scratch?
>
>
> I suppose it is just a simple bug with absent NULL pointer check. Here
> is
> original code:
>
> /* Duplicate the loop tree, if available and wanted. */
> if (loops_for_fn (src_cfun) != NULL
> && current_loops != NULL)
> {
> copy_loops (id, entry_block_map->loop_father,
> get_loop (src_cfun, 0));
> /* Defer to cfgcleanup to update loop-father fields of
> basic-blocks. */
> loops_state_set (LOOPS_NEED_FIXUP);
> }
>
> /* If the loop tree in the source function needed fixup, mark the
>destination loop tree for fixup, too. */
> if (loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
> loops_state_set (LOOPS_NEED_FIXUP);
>
> As you may see we have check for absent loops structure in the first
> if-statement and no check in the second one. I hit segfault and added
> the
> check.
Downthread you indicated you're not in SSA form which might explain the
inconsistency here. If so, then we need to make sure that the loop & df
structures do get set up properly later.
>>>
>>>
>>> That is what init_data_structures pass will do for us as Richard pointed.
>>> Right?
>>
>>
>> loops are set up during the CFG construction and thus are available
>> everywhere.
>
> Which would argue that the hunk that checks for the loop tree's existence
> before accessing it should not be needed. Ilya -- is it possible you hit
> this prior to Richi's work to build the loop structures as part of CFG
> construction and maintain them throughout compilation.
That's likely. It's still on my list of janitor things to do to remove all
those if (current_loops) checks ...
>> the df structures are set up in init_data_structures pass which is run
>> before
>> going into SSA form (I'd like to somehow cleanup that area).
>
> OK. So this part should be approved since we've established this code is
> running prior to going into SSA form.
Agreed.
Thanks,
Richard.
> jeff
>
Re: [PATCH, Pointer Bounds Checker 13/x] Early versioning
On 04 Jun 11:59, Richard Biener wrote:
> On Wed, Jun 4, 2014 at 8:46 AM, Jeff Law wrote:
> > On 06/03/14 03:29, Richard Biener wrote:
> >>
> >> On Tue, Jun 3, 2014 at 7:55 AM, Ilya Enkovich
> >> wrote:
> >>>
> >>> 2014-06-02 21:27 GMT+04:00 Jeff Law :
>
> On 06/02/14 04:48, Ilya Enkovich wrote:
> >>
> >>
> >> Hmm, so if I understand things correctly, src_fun has no loop
> >> structures attached, thus there's nothing to copy. Presumably at
> >> some later point we build loop structures for the copy from scratch?
> >
> >
> > I suppose it is just a simple bug with absent NULL pointer check. Here
> > is
> > original code:
> >
> > /* Duplicate the loop tree, if available and wanted. */
> > if (loops_for_fn (src_cfun) != NULL
> > && current_loops != NULL)
> > {
> > copy_loops (id, entry_block_map->loop_father,
> > get_loop (src_cfun, 0));
> > /* Defer to cfgcleanup to update loop-father fields of
> > basic-blocks. */
> > loops_state_set (LOOPS_NEED_FIXUP);
> > }
> >
> > /* If the loop tree in the source function needed fixup, mark the
> >destination loop tree for fixup, too. */
> > if (loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
> > loops_state_set (LOOPS_NEED_FIXUP);
> >
> > As you may see we have check for absent loops structure in the first
> > if-statement and no check in the second one. I hit segfault and added
> > the
> > check.
>
>
> Downthread you indicated you're not in SSA form which might explain the
> inconsistency here. If so, then we need to make sure that the loop & df
> structures do get set up properly later.
> >>>
> >>>
> >>> That is what init_data_structures pass will do for us as Richard pointed.
> >>> Right?
> >>
> >>
> >> loops are set up during the CFG construction and thus are available
> >> everywhere.
> >
> > Which would argue that the hunk that checks for the loop tree's existence
> > before accessing it should not be needed. Ilya -- is it possible you hit
> > this prior to Richi's work to build the loop structures as part of CFG
> > construction and maintain them throughout compilation.
>
> That's likely. It's still on my list of janitor things to do to remove all
> those if (current_loops) checks ...
I tried to remove this loops check and got no failures this time. So, here is
a new patch version.
Bootstrapped and tested on linux-x86_64.
Thanks,
Ilya
--
gcc/
2014-06-05 Ilya Enkovich
* tree-inline.c (tree_function_versioning): Check DF info existence
before accessing it.
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 4293241..2972346 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -5350,8 +5350,9 @@ tree_function_versioning (tree old_decl, tree new_decl,
DECL_ARGUMENTS (new_decl) = DECL_ARGUMENTS (old_decl);
initialize_cfun (new_decl, old_decl,
old_entry_block->count);
- DECL_STRUCT_FUNCTION (new_decl)->gimple_df->ipa_pta
-= id.src_cfun->gimple_df->ipa_pta;
+ if (DECL_STRUCT_FUNCTION (new_decl)->gimple_df)
+DECL_STRUCT_FUNCTION (new_decl)->gimple_df->ipa_pta
+ = id.src_cfun->gimple_df->ipa_pta;
/* Copy the function's static chain. */
p = DECL_STRUCT_FUNCTION (old_decl)->static_chain_decl;
Re: [PATCH, Pointer Bounds Checker 13/x] Early versioning
On Thu, Jun 5, 2014 at 1:18 PM, Ilya Enkovich wrote:
> On 04 Jun 11:59, Richard Biener wrote:
>> On Wed, Jun 4, 2014 at 8:46 AM, Jeff Law wrote:
>> > On 06/03/14 03:29, Richard Biener wrote:
>> >>
>> >> On Tue, Jun 3, 2014 at 7:55 AM, Ilya Enkovich
>> >> wrote:
>> >>>
>> >>> 2014-06-02 21:27 GMT+04:00 Jeff Law :
>>
>> On 06/02/14 04:48, Ilya Enkovich wrote:
>> >>
>> >>
>> >> Hmm, so if I understand things correctly, src_fun has no loop
>> >> structures attached, thus there's nothing to copy. Presumably at
>> >> some later point we build loop structures for the copy from scratch?
>> >
>> >
>> > I suppose it is just a simple bug with absent NULL pointer check. Here
>> > is
>> > original code:
>> >
>> > /* Duplicate the loop tree, if available and wanted. */
>> > if (loops_for_fn (src_cfun) != NULL
>> > && current_loops != NULL)
>> > {
>> > copy_loops (id, entry_block_map->loop_father,
>> > get_loop (src_cfun, 0));
>> > /* Defer to cfgcleanup to update loop-father fields of
>> > basic-blocks. */
>> > loops_state_set (LOOPS_NEED_FIXUP);
>> > }
>> >
>> > /* If the loop tree in the source function needed fixup, mark the
>> >destination loop tree for fixup, too. */
>> > if (loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
>> > loops_state_set (LOOPS_NEED_FIXUP);
>> >
>> > As you may see we have check for absent loops structure in the first
>> > if-statement and no check in the second one. I hit segfault and added
>> > the
>> > check.
>>
>>
>> Downthread you indicated you're not in SSA form which might explain the
>> inconsistency here. If so, then we need to make sure that the loop & df
>> structures do get set up properly later.
>> >>>
>> >>>
>> >>> That is what init_data_structures pass will do for us as Richard pointed.
>> >>> Right?
>> >>
>> >>
>> >> loops are set up during the CFG construction and thus are available
>> >> everywhere.
>> >
>> > Which would argue that the hunk that checks for the loop tree's existence
>> > before accessing it should not be needed. Ilya -- is it possible you hit
>> > this prior to Richi's work to build the loop structures as part of CFG
>> > construction and maintain them throughout compilation.
>>
>> That's likely. It's still on my list of janitor things to do to remove all
>> those if (current_loops) checks ...
>
> I tried to remove this loops check and got no failures this time. So, here
> is a new patch version.
>
> Bootstrapped and tested on linux-x86_64.
Ok (you can commit this now).
Thanks,
Richard.
> Thanks,
> Ilya
> --
> gcc/
>
> 2014-06-05 Ilya Enkovich
>
> * tree-inline.c (tree_function_versioning): Check DF info existence
> before accessing it.
>
>
> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
> index 4293241..2972346 100644
> --- a/gcc/tree-inline.c
> +++ b/gcc/tree-inline.c
> @@ -5350,8 +5350,9 @@ tree_function_versioning (tree old_decl, tree new_decl,
>DECL_ARGUMENTS (new_decl) = DECL_ARGUMENTS (old_decl);
>initialize_cfun (new_decl, old_decl,
>old_entry_block->count);
> - DECL_STRUCT_FUNCTION (new_decl)->gimple_df->ipa_pta
> -= id.src_cfun->gimple_df->ipa_pta;
> + if (DECL_STRUCT_FUNCTION (new_decl)->gimple_df)
> +DECL_STRUCT_FUNCTION (new_decl)->gimple_df->ipa_pta
> + = id.src_cfun->gimple_df->ipa_pta;
>
>/* Copy the function's static chain. */
>p = DECL_STRUCT_FUNCTION (old_decl)->static_chain_decl;
Re: [PATCH, Pointer Bounds Checker 13/x] Early versioning
2014-06-05 15:58 GMT+04:00 Richard Biener :
> On Thu, Jun 5, 2014 at 1:18 PM, Ilya Enkovich wrote:
>> On 04 Jun 11:59, Richard Biener wrote:
>>> On Wed, Jun 4, 2014 at 8:46 AM, Jeff Law wrote:
>>> > On 06/03/14 03:29, Richard Biener wrote:
>>> >>
>>> >> On Tue, Jun 3, 2014 at 7:55 AM, Ilya Enkovich
>>> >> wrote:
>>> >>>
>>> >>> 2014-06-02 21:27 GMT+04:00 Jeff Law :
>>>
>>> On 06/02/14 04:48, Ilya Enkovich wrote:
>>> >>
>>> >>
>>> >> Hmm, so if I understand things correctly, src_fun has no loop
>>> >> structures attached, thus there's nothing to copy. Presumably at
>>> >> some later point we build loop structures for the copy from scratch?
>>> >
>>> >
>>> > I suppose it is just a simple bug with absent NULL pointer check.
>>> > Here
>>> > is
>>> > original code:
>>> >
>>> > /* Duplicate the loop tree, if available and wanted. */
>>> > if (loops_for_fn (src_cfun) != NULL
>>> > && current_loops != NULL)
>>> > {
>>> > copy_loops (id, entry_block_map->loop_father,
>>> > get_loop (src_cfun, 0));
>>> > /* Defer to cfgcleanup to update loop-father fields of
>>> > basic-blocks. */
>>> > loops_state_set (LOOPS_NEED_FIXUP);
>>> > }
>>> >
>>> > /* If the loop tree in the source function needed fixup, mark the
>>> >destination loop tree for fixup, too. */
>>> > if (loops_for_fn (src_cfun)->state & LOOPS_NEED_FIXUP)
>>> > loops_state_set (LOOPS_NEED_FIXUP);
>>> >
>>> > As you may see we have check for absent loops structure in the first
>>> > if-statement and no check in the second one. I hit segfault and added
>>> > the
>>> > check.
>>>
>>>
>>> Downthread you indicated you're not in SSA form which might explain the
>>> inconsistency here. If so, then we need to make sure that the loop &
>>> df
>>> structures do get set up properly later.
>>> >>>
>>> >>>
>>> >>> That is what init_data_structures pass will do for us as Richard
>>> >>> pointed.
>>> >>> Right?
>>> >>
>>> >>
>>> >> loops are set up during the CFG construction and thus are available
>>> >> everywhere.
>>> >
>>> > Which would argue that the hunk that checks for the loop tree's existence
>>> > before accessing it should not be needed. Ilya -- is it possible you hit
>>> > this prior to Richi's work to build the loop structures as part of CFG
>>> > construction and maintain them throughout compilation.
>>>
>>> That's likely. It's still on my list of janitor things to do to remove all
>>> those if (current_loops) checks ...
>>
>> I tried to remove this loops check and got no failures this time. So, here
>> is a new patch version.
>>
>> Bootstrapped and tested on linux-x86_64.
>
> Ok (you can commit this now).
Thanks! Committed to trunk
Ilya
>
> Thanks,
> Richard.
>
>> Thanks,
>> Ilya
>> --
>> gcc/
>>
>> 2014-06-05 Ilya Enkovich
>>
>> * tree-inline.c (tree_function_versioning): Check DF info existence
>> before accessing it.
>>
>>
>> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
>> index 4293241..2972346 100644
>> --- a/gcc/tree-inline.c
>> +++ b/gcc/tree-inline.c
>> @@ -5350,8 +5350,9 @@ tree_function_versioning (tree old_decl, tree new_decl,
>>DECL_ARGUMENTS (new_decl) = DECL_ARGUMENTS (old_decl);
>>initialize_cfun (new_decl, old_decl,
>>old_entry_block->count);
>> - DECL_STRUCT_FUNCTION (new_decl)->gimple_df->ipa_pta
>> -= id.src_cfun->gimple_df->ipa_pta;
>> + if (DECL_STRUCT_FUNCTION (new_decl)->gimple_df)
>> +DECL_STRUCT_FUNCTION (new_decl)->gimple_df->ipa_pta
>> + = id.src_cfun->gimple_df->ipa_pta;
>>
>>/* Copy the function's static chain. */
>>p = DECL_STRUCT_FUNCTION (old_decl)->static_chain_decl;
