This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, Pointer Bounds Checker 13/x] Early versioning


2014-06-05 15:58 GMT+04:00 Richard Biener <richard.guenther@gmail.com>:
> On Thu, Jun 5, 2014 at 1:18 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> On 04 Jun 11:59, Richard Biener wrote:
>>> On Wed, Jun 4, 2014 at 8:46 AM, Jeff Law <law@redhat.com> wrote:
>>> > On 06/03/14 03:29, Richard Biener wrote:
>>> >>
>>> >> On Tue, Jun 3, 2014 at 7:55 AM, Ilya Enkovich <enkovich.gnu@gmail.com>
>>> >> wrote:
>>> >>>
>>> >>> 2014-06-02 21:27 GMT+04:00 Jeff Law <law@redhat.com>:
>>> >>>>
>>> >>>> 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  <ilya.enkovich@intel.com>
>>
>>         * 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;


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]