2014-06-02 15:56 GMT+04:00 Richard Biener <richard.guent...@gmail.com>:
> 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?
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
>>>
