Hi, On Wed, Feb 22, 2017 at 11:11:14AM +0100, Martin Jambor wrote: > Hello, > > this is a fix for PR 78140 which is about LTO WPA of Firefox taking > 1GB memory more than gcc 6. > > It works by reusing the ipa_bits and value_range that we previously > had directly in jump functions and which are just too big to be > allocated for each actual argument in all of Firefox. Reusing is > achieved by two hash table traits derived from ggc_cache_remove which > apparently has been created just for this purpose and once I > understood them they made my life a lot easier. In future, I will > have a look at applying this method to other parts of jump functions > as well. > > According to my measurements, the patch saves about 1.2 GB of memory. > The problem is that some change last week (between revision 245382 and > 245595) has more than invalidated this: > > | compiler | WPA mem (GB) | > |---------------------+--------------| > | gcc 6 branch | 3.86 | > | trunk rev. 245382 | 5.21 | > | patched rev. 245382 | 4.06 | > | trunk rev. 245595 | 6.59 | > | patched rev. 245595 | 5.25 | >
I have found out that previously I was comparing a build with --enable-gather-detailed-mem-stats with one configured without it, which explains the difference. So the further 1GB increase was fortunately only a mistake in measurements, the real data (without detailed memory statistics overhead) are below and the proposed patch does fix lion's share of the gcc 7 memory consumption increase: | compiler | wpa mem (KB) | wpa mem (GB) | |---------------------+--------------+--------------| | gcc 6 branch | 4046451 | 3.86 | | trunk rev. 245382 | 5468227 | 5.21 | | patched rev. 245382 | 4255799 | 4.06 | | trunk rev. 245595 | 5452515 | 5.20 | | patched rev. 245595 | 4240379 | 4.04 | I suppose we can look for the remaining ~200MB after the patch is approved. Sorry for the noise, Martin > (I have verified this by Martin's way of measuring things.) I will > try to bisect what commit has caused the increase. Still, the patch > helps a lot. > > There is one thing in the patch that intrigues me, I do not understand > why I had to mark value_range with GTY((for_user)) - as opposed to > just GTY(()) that was there before - whereas ipa_bits does not need > it. If anyone could enlighten me, that would be great. But I suppose > this is not an indication of anything being wrong under the hood. > > I have bootstrapped and LTO-bootstrapped the patch on x86_64-linux and > also bootstrapped (C, C++ and Fortran) on an aarch64 and i686 cfarm > machine. I have also LTO-built Firefox with the patch and used it to > browse for a while and it seemed fine. > > OK for trunk? > > Thanks, > > Martin > > > 2017-02-20 Martin Jambor <mjam...@suse.cz> > > PR lto/78140 > * ipa-prop.h (ipa_bits): Removed field known. > (ipa_jump_func): Removed field vr_known. Changed fields bits and m_vr > to pointers. Adjusted their comments to warn about their sharing. > (ipcp_transformation_summary): Change bits to a vector of pointers. > (ipa_check_create_edge_args): Moved to ipa-prop.c, declare. > (ipa_get_ipa_bits_for_value): Declare. > * tree-vrp.h (value_range): Mark as GTY((for_user)). > * ipa-prop.c (ipa_bit_ggc_hash_traits): New. > (ipa_bits_hash_table): Likewise. > (ipa_vr_ggc_hash_traits): Likewise. > (ipa_vr_hash_table): Likewise. > (ipa_print_node_jump_functions_for_edge): Adjust for bits and m_vr > being pointers and vr_known being removed. > (ipa_set_jf_unknown): Likewise. > (ipa_get_ipa_bits_for_value): New function. > (ipa_set_jfunc_bits): Likewise. > (ipa_get_value_range): New overloaded functions. > (ipa_set_jfunc_vr): Likewise. > (ipa_compute_jump_functions_for_edge): Use the above functions to > construct bits and vr parts of jump functions. > (ipa_check_create_edge_args): Move here from ipa-prop.h, also allocate > ipa_bits_hash_table and ipa_vr_hash_table if they do not already > exist. > (ipcp_grow_transformations_if_necessary): Also allocate > ipa_bits_hash_table and ipa_vr_hash_table if they do not already > exist. > (ipa_node_params_t::duplicate): Do not copy bits, just pointers to > them. Fix too long lines. > (ipa_write_jump_function): Adjust for bits and m_vr being pointers and > vr_known being removed. > (ipa_read_jump_function): Use new setter functions to construct bits > and vr parts of jump functions or set them to NULL. > (write_ipcp_transformation_info): Adjust for bits being pointers. > (read_ipcp_transformation_info): Likewise. > (ipcp_update_bits): Likewise. Fix excessively long lines a trailing > space. > Include gt-ipa-prop.h. > * ipa-cp.c (propagate_bits_across_jump_function): Adjust for bits > being pointers. > (ipcp_store_bits_results): Likewise. > (propagate_vr_across_jump_function): Adjust for m_vr being a pointer. > Do not write to existing jump functions but use a temporary instead.
