Hi,

On Mon, Feb 17, 2014 at 09:40:40AM +0100, Jan Hubicka wrote:
> Hi,
> Chromium LTO build ICEs on bogus get_binfo_at_offset call. This is caused by
> updating pasto bug in update_jump_functions_after_inlining.
> 
> While looking for it I noticed we have other issues here. In particular, when
> combining -fno-devirtualize and -fdevirtualize units, we get all jump fuctions
> as type preserved.  This is because detect_type_change returns false when it
> decides detection is not needed. It really should return true. Otherwise we 
> will
> end up propagating invalid types through the -fno-devirtualize function and 
> produce
> wrong code at LTO.
> 
> I also added some sanity checking to jump functions while hunting the bug that
> uncovered another problem in compute_complex_assign_jump_func and
> compute_complex_ancestor_jump_func that are still confusing type of parameter
> (that is pointer) to type of object passed by reference.
> 
> I also made us to not save the type to ipa_set_ancestor_jf when type is not
> preserved, since it will never be used for enything useful. 
> ipa_get_jf_ancestor_result
> uses to build it the updated address, but we can safely use pointer_type_node,
> because it will be converted at the substitution time anyway.
> 
> Bootstrapped/regtested x86_64-linux, will commit it tomorrow after some 
> furhter testing
> and unless Martin stops me ;)
> 
> Honza
> 
>       * ipa-cp.c (ipa_get_jf_ancestor_result): Allow type to be NULL.
>       * ipa-prop.c (ipa_set_jf_known_type): Do not set anyting when
>       we do not devirtualize.
>       (ipa_set_ancestor_jf): Likewise; do not record type when we do not
>       devirtualize.
>       (detect_type_change): Signalize type change when giving up on
>       detection.
>       (compute_complex_assign_jump_func): Fix call of ipa_set_ancestor_jf.
>       (compute_complex_ancestor_jump_func): Likewise.
>       (update_jump_functions_after_inlining): Fix combining of PASS_THROUGH
>       and KNOWN_TYPE.

This is fine, except that if we allow having NULL as the type of
ancestor jump functions, this should be stated and explained in a
comment in ipa_ancestor_jf_data in ipa-prop.h.

Thanks,

Martin

Reply via email to