This is the mail archive of the
mailing list for the GCC project.
Re: [patch] ipa/ipcp partitioning
- From: Steven Bosscher <stevenb at suse dot de>
- To: Olga Golovanevsky <OLGA at il dot ibm dot com>
- Cc: jh at suse dot cz, Razya Ladelsky <RAZYA at il dot ibm dot com>, gcc-patches at gcc dot gnu dot org
- Date: Tue, 17 May 2005 20:06:40 +0200
- Subject: Re: [patch] ipa/ipcp partitioning
- References: <OFB1C29DC7.78674083-ONC2257004.003EA324-C2257004.004130F3@il.ibm.com>
On Tuesday 17 May 2005 13:52, Olga Golovanevsky wrote:
> This patch is a ipa_ipcp.[ch] files partitioning/renaming.
> File ipa-prop.c will contain commonly used by ipcp and ipaa functions,
> that are now also declared in ipa-prop.h, while file ipcp.c will be
> specific for constant propagation.
> ok for tree-profiling-branch?
> 2005-05-17 Olga Golovanevsky <firstname.lastname@example.org>
> * ipa_prop.c: Deleted.
> * ipa_prop.h: Deleted.
> * ipa-prop.c: New file. It contains all ipa_ functions
> that were in ipa_prop.c. They are non-static now.
> * ipa-prop.h: New file, renamed form ipa_prop.h. Declarations
> of all ipa_ functions from ipa-prop.c are added.
> * ipcp.c: Contains all ipcp_ functions that were in ipa_prop.c
> * Makefile.in (ipa-prop.c, ipa-prop.h): Renamed from ipa_prop.c,
> (ipcp.o): New target. Added to OBJS-archive.
> (ipcp.c): Added to GTFILES.
> * cgraphunit.c: Includes ipa-prop.h instead of ipa_prop.h
> * tree-inline.c: Similar to cgraphunit.c.
Overall, the split-up looks very reasonable. /me <-- happy :-)
But there are still a few things that need fixing...
There are still a few places where you refer to IPCP, you probably want
to fix up such names/comments, e.g.
/* Creates the worklist for ipcp_iterate_stage(). */
/* Frees the ipcp_method's IPCP data structures. */
You've also been a bit over-zealous with removing static. Functions
like ipa_create_methodlist_node are only used in ipa-prop.c (and that
should probably stay that way) so they can (and should) still be
The comment in the ipa-prop.c file about what the file provides is
/* This file contains interfaces common for different
interprocedural analyses. */
This is not very informative. You should document the interfcaces the
provides here. Define those interfaces, this is _so_ important for
other people to understand what is going on! (I believe we should later
on also add an ipa.texi to the internals documentation, but... later ;-).
Nit: The copyright years should also be fixed on ipa-prop.h at least.
Thanks for working on this,