Hi, On Sat, Aug 29 2020, Jan Hubicka wrote: >> Hi, >> >> this large patch is a semi-mechanical change which aims to replace >> uses of separate vectors about known scalar values (usually called >> known_vals or known_csts), known aggregate values (known_aggs), known >> virtual call contexts (known_contexts) and known value >> ranges (known_value_ranges) with uses of one type called >> ipa_call_arg_values. The main benefit is bringing down the number of >> arguments that various call context evaluating functions have. >> Because the new type uses auto_vecs deallocation is simpler and it >> also takes advantage of storage within the auto_vecs themselves, >> eliminating the need for allocating memory when analyzing most >> functions. >> >> The one place where this patch is not mechanical is the >> node_context_cache_entry RCU cache. Before the elements contained >> directly ipa_call_context instances which however owned their vectors, >> they did not share them with their users like non-cache contexts. Now >> the vectors are behind the pointer which means ipa_call_arg_values >> would have to be allocated, for many functions at once, and so it >> could not make use of auto_vecs with generous internal storage. >> >> I avoided both by reworking the cache to contain copies of all fields >> o interest of ipa_call_context, including the interesting vectors. >> This makes the type a bit more verbose but the main functionality, >> storing to the cache and comparing a context with a cache entry, >> remained very similar. > > Can you pleae benchmark this on building Firfox or something similar? > I was running into two problems here. First was overhead of malloc > allocations and that is reason why I am trying to not allocate the > vector when not necessary and also place them on stack rather then heap > for those having short lifetime (not being in cache). > > Second problem is memory use - we may end up with quite many contextes > since calls are very numerous. >
I can but the patch does not change where the vectors are allocated and does not store more stuff to cache than the current code. In fact, because the new cache does not store m_node, it saves one pointer. When I wrote the patch "copies of all fields of interest of ipa_call_context" I meant in the source code of GCC, from one class into another, not when the cache operates. The storing mechanism was created by modifying the current duplicate_from so that it operates on two types instead of one, it still meticulously makes sure it copies only useful data and all that. But sure, I will double check the series does not increase memory use. Martin
