Hi, I believe the general consensus is that lazy call graph node creation is no longer a good idea and a few of us have seen bugs caused by a creation of a node when we did not expect it. Therefore I embarked on getting rid of it. In the process I quickly realized it would be difficult to do that completely but it is certainly possible to weed it out of most places where it is not necessary. We can then tackle the remaining places separately.
These patches are meant to be applied on top of http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01170.html The first patch in the series removes the cgraph_node function. Most of its callers should use already existing cgraph_get_node instead, for the others there are new functions: - craph_create_node will create a node but fail on assert if there already is a node for the declaration - cgraph_get_create_node is the new lazy node creation. I did not want to continue using cgraph_node name because I really thought the it was unfortunate and the new name will also help us to discourage potential new uses and isolate those that we might want to convert to a non-lazy way of creating nodes later. - cgraph_do_get_node is a simple wrapper around cgraph_get_node which checking_asserts that the return value is not NULL. I found it very useful while developing the patch and think it can be sometimes useful even in the longer term, for example when the result is stored in some data structure which is to be used later. If the general consensus is that we don't want it, it is easy to remove it from the patch. When I was processing calls to cgraph_node I first tried to determine if it is possible for cgraph_get_node to return NULL. If it was, I either lazily-created the node or assumed the default node properties depending on the nature of the call. If I thought NULL should not be returned, I used cgraph_do_get_node if the code would not immediately segfault. The places which still need lazy node construction can be roughly divided into the following categories: - Stuff called from the front ends. Most noticeably, nested functions are organized in a tree of call graph nodes for some time, the leaves are entered first and the cgraph_(create_)node creates the encapsulation one for DECL_CONTEXT. And then there are aliases. And perhaps more stuff I already forgot about. In most cases, keeping it is not harmful and even sensible. - Almost all of cgraphbuild.c. But that basically makes sense. - Sometimes the decl of call statement is a builtin that is newly created by folding. If it does not have a node at that point, we might trigger an assert. I have tried to verify diligently that NULL cannot be returned at places where I don't handle it and of course I tested the changes thoroughly. Nevertheless, there are few places where I'd appreciate if someone confirmed my conclusions. These are: - lower_emutls_function_body honk and - hunks in ipa-struct-reorg.c and ipa-type-escape.c ...just in case anyone really cared ;-) The patch is not very complex but I understand the scope it changes is vast. In order to ease its assessment, all changes in front-ends change calls to cgraph_node to calls to cgraph_get_create_node (except an obvious assert in java). I hope that this will mean the big first patch will eventually not require approval of individual front-end maintainers. There are three simple subsequent patches which change the calls in front-end specific code but those are much smaller and not that crucial to the whole effort. I have successfully bootstrapped and tested "all,ada" on x86_64-linux and "c,c++,fortran" on i686. I have successfully checked "c,c++,fortran" on ia64 and (an almost identical patch) on sparc64. Sorry for the long email and long patch to review. I understand there will be comments and suggestions and I thank in advance for them. At the same time I think it is really a good idea to commit something like this early in stage 1. Tomorrow I'm leaving for a four day vacation (I'll be back on Thursday) so please be patient when waiting for my reply. Thanks! Martin
