On Mon, Jun 2, 2014 at 12:48 PM, Ilya Enkovich <enkovich....@gmail.com> 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 <ilya.enkov...@intel.com>
>> >
>> > * 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
>>
