diff mbox

IPA cleanups and assorted cleanups

Message ID 20120913122909.GA3452@atrey.karlin.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka Sept. 13, 2012, 12:29 p.m. UTC 
> Hello,
> 
> This patch cleans up some things that annoyed me in ipa-reference.c
> and ipa-pure-const.c. These two passes are very important but they
> show all the signs of being developed when GCC's IPA infrastructure
> was still (or at least even more than today) in its infancy: Walking

yeah, those really developed from really weird initial implementations and was
getting updated for a long while..  Cleaning this up was on my TODO for a
while, but never got high enough.  Thanks for looking into it.

> 2. The management of sets is clarified using functions with intuitive
> names that also help reduce the cost of the transitive closure by
> detecting when a set union(X,Y) is the maximum set (reducing the
> memory footprint from 3 GB to just ~120MB for a large C++ test case)

nice... I tought it was already done :))

Comments

Steven Bosscher Sept. 13, 2012, 1:40 p.m. UTC | #1 
On Thu, Sep 13, 2012 at 2:29 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> +/* Get the set of nodes for the cycle in the reduced call graph starting
> +   from NODE.  */
> +
> +VEC (cgraph_node_p, heap) *
> +ipa_get_nodes_in_cycle (struct cgraph_node *node)
>
> I never really like the api of SCC searching that made user to walk across AUX
> pointer.
>
> I however also do not like allocating a temporary vector and requiring user to
> mind to free it just to add abstraction about single linked list walk.

I think this new ipa_get_nodes_in_cycle thing is an improvement over
what there is now, but I agree that something better is necessary in
the long term. Not just for visiting nodes in a cycle, but also for
how a cycle is represented. Linking via the aux pointer wouldn't have
been my choice to begin with...

(FWIW, I also dislike the get_loop_body stuff. Loop nodes should just
be a set directly available from the loop info without a CFG DFS.)


>  What
> about adding convenient iterator API, especially now when we have the C++
> wonderland?

Maybe later. But I'll wait with C++ iterators until someone has put an
example in the trunk that I can copy-and-paste :-)

Ciao!
Steven
diff mbox

Patch

Index: ipa-utils.c
===================================================================
--- ipa-utils.c	(revision 191214)
+++ ipa-utils.c	(working copy)
@@ -154,8 +154,11 @@  searchc (struct searchc_env* env, struct cgraph_no
 
 /* Topsort the call graph by caller relation.  Put the result in ORDER.
 
-   The REDUCE flag is true if you want the cycles reduced to single nodes.  Set
-   ALLOW_OVERWRITABLE if nodes with such availability should be included.
+   The REDUCE flag is true if you want the cycles reduced to single nodes.
+   You can use ipa_get_nodes_in_cycle to obtain a vector containing all real
+   call graph nodes in a reduced node.
+
+   Set ALLOW_OVERWRITABLE if nodes with such availability should be included.
    IGNORE_EDGE, if non-NULL is a hook that may make some edges insignificant
    for the topological sort.   */
 
@@ -231,6 +234,23 @@  ipa_free_postorder_info (void)
     }
 }
 
+/* Get the set of nodes for the cycle in the reduced call graph starting
+   from NODE.  */
+
+VEC (cgraph_node_p, heap) *
+ipa_get_nodes_in_cycle (struct cgraph_node *node)

I never really like the api of SCC searching that made user to walk across AUX
pointer.

I however also do not like allocating a temporary vector and requiring user to
mind to free it just to add abstraction about single linked list walk.  What
about adding convenient iterator API, especially now when we have the C++
wonderland?

Thanks,
Honza