This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] ipa/ipcp partitioning


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?
>
> olga
>
> 2005-05-17  Olga Golovanevsky  <olga@il.ibm.com>
>
>       * 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,
>       ipa_prop.h.
>       (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().  */
inline ipa_methodlist_p
ipa_create_methodlist_node (void) 
{

/* Frees the ipcp_method's IPCP data structures.  */
void 
ipa_free (void)
{

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
static inline.

The comment in the ipa-prop.c file about what the file provides is
below par:

/* 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,

Gr.
Steven




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]