[RFC] Bug lto/78140

[kugan]

Hi Richard,

On 30/01/17 21:08, Richard Biener wrote:
> On Mon, Jan 30, 2017 at 12:23 AM, kugan
> <kugan.vivekanandarajah@linaro.org> wrote:
>> Hi All,
>>
>> As suggested by Richard in the PR, I tried to implement variable size
>> structures for VR as shown in attached patch. That is, I changed ipa-prop.h
>> to:
>>
>> diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
>> index 93a2390c..acab2aa 100644
>> --- a/gcc/ipa-prop.h
>> +++ b/gcc/ipa-prop.h
>> @@ -157,13 +157,15 @@ struct GTY(()) ipa_bits
>>  };
>>
>>  /* Info about value ranges.  */
>> -struct GTY(()) ipa_vr
>> +struct GTY ((variable_size)) ipa_vr
>>  {
>>    /* The data fields below are valid only if known is true.  */
>>    bool known;
>>    enum value_range_type type;
>> -  wide_int min;
>> -  wide_int max;
>> +  /* Minimum and maximum.  */
>> +  TRAILING_WIDE_INT_ACCESSOR (min, ints, 0)
>> +  TRAILING_WIDE_INT_ACCESSOR (max, ints, 1)
>> +  trailing_wide_ints <2> ints;
>>  };
>>
>>  /* A jump function for a callsite represents the values passed as actual
>> @@ -525,7 +527,7 @@ struct GTY(()) ipcp_transformation_summary
>>    /* Known bits information.  */
>>    vec<ipa_bits, va_gc> *bits;
>>    /* Value range information.  */
>> -  vec<ipa_vr, va_gc> *m_vr;
>> +  vec<ipa_vr *, va_gc> *m_vr;
>>  };
>>
>>  void ipa_set_node_agg_value_chain (struct cgraph_node *node,
>>
>> However, I am running into error when I do LTO bootstrap that memory seems
>> to have deallocated by the garbage collector. Since we have the reference to
>> the memory allocated by ggc_internal_alloc in the vector (m_vr), I thought
>> it will not be deallocated. But during the bootstrap, when in
>> ipa_node_params_t::duplicate, it seems to have been deallocated as shown in
>> the back trace. I dont understand internals of gc in gcc so any help is
>> appreciated.
>>
>>
>> lto1: internal compiler error: Segmentation fault
>> 0xdedc4b crash_signal
>>         ../../gcc/gcc/toplev.c:333
>> 0xb46680 ipa_node_params_t::duplicate(cgraph_node*, cgraph_node*,
>> ipa_node_params*, ipa_node_params*)
>>         ../../gcc/gcc/ipa-prop.c:3819
>> 0xb306a3
>> function_summary<ipa_node_params*>::symtab_duplication(cgraph_node*,
>> cgraph_node*, void*)
>>         ../../gcc/gcc/symbol-summary.h:187
>> 0x85aba7 symbol_table::call_cgraph_duplication_hooks(cgraph_node*,
>> cgraph_node*)
>>         ../../gcc/gcc/cgraph.c:488
>> 0x8765bf cgraph_node::create_clone(tree_node*, long, int, bool,
>> vec<cgraph_edge*, va_heap, vl_ptr>, bool, cgraph_node*, bitmap_head*, char
>> const*)
>>         ../../gcc/gcc/cgraphclones.c:522
>> 0x166fb3b clone_inlined_nodes(cgraph_edge*, bool, bool, int*, int)
>>         ../../gcc/gcc/ipa-inline-transform.c:227
>> 0x166fbd7 clone_inlined_nodes(cgraph_edge*, bool, bool, int*, int)
>>         ../../gcc/gcc/ipa-inline-transform.c:242
>> 0x1670893 inline_call(cgraph_edge*, bool, vec<cgraph_edge*, va_heap,
>> vl_ptr>*, int*, bool, bool*)
>>         ../../gcc/gcc/ipa-inline-transform.c:449
>> 0x1665bd3 inline_small_functions
>>         ../../gcc/gcc/ipa-inline.c:2024
>> 0x1667157 ipa_inline
>>         ../../gcc/gcc/ipa-inline.c:2434
>> 0x1667fa7 execute
>>         ../../gcc/gcc/ipa-inline.c:2845
>>
>>
>> I tried similar think without variable structure like:
>> diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
>> index 93a2390c..b0cc832 100644
>> --- a/gcc/ipa-prop.h
>> +++ b/gcc/ipa-prop.h
>> @@ -525,7 +525,7 @@ struct GTY(()) ipcp_transformation_summary
>>    /* Known bits information.  */
>>    vec<ipa_bits, va_gc> *bits;
>>    /* Value range information.  */
>> -  vec<ipa_vr, va_gc> *m_vr;
>> +  vec<ipa_vr *, va_gc> *m_vr;
>>  };
>>
>> This also has the same issue so I don't think it has anything to do with
>> variable structure.
>
> You have to debug that detail yourself but I wonder why the transformation
> summary has a different representation than the jump function (and I think
> the jump function size is the issue).
>
> The JF has
>
>   /* Information about zero/non-zero bits.  */
>   struct ipa_bits bits;
>
>   /* Information about value range, containing valid data only when vr_known is
>      true.  */
>   value_range m_vr;
>   bool vr_known;
>
> with ipa_bits having two widest_ints and value_range having two trees
> and an unused bitmap and ipa_vr having two wide_ints (widest_ints are
> smaller!).
>
We now have ipa_vr (with wide_int) for ipcp_transaction_summary. 
value_range is used in jump functions as it uses tree-vrp for most of 
the handling. Attached patch uses variable_structure for ipa_vr. A 
version of this patch passed regression and lto bootstrapped (before I 
separated the part that prevented testing and posted that separately in 
https://gcc.gnu.org/ml/gcc-patches/2017-02/msg00103.html). Does this 
approach looks OK? I will do full testing and post again based on the 
feedback.

I am also going to do the same for ipa-bits based on the feedback. Do 
you also want to do variable structure with widest_ints?

Thanks,
Kugan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-p2.patch
Type: text/x-diff
Size: 8294 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20170202/ecbef25c/attachment.bin>