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: [PING] Interprocedural Constant Propagation for mainline


On Thursday 16 June 2005 17:09, Razya Ladelsky wrote:
> Hello,
>
> This patch implements Interprocedural constant propagation (IPCP).
(...)
> Comments are welcome.

Too bad nobody has commented on this large body of work.  This is the
first patch of at least two for a stage 2 project on Mark's list.  The
end of stage 2 is already on July 8.  The first time IPCP was sumbitted
is already two weeks ago now, but if nobody with approval rights is
going to look at it, us folks hacking on the tree-profiling-branch have
a serious problem... :-/


Anyway, Razya, I still have a few comments...

/* Return whether TYPE is a constant type.  */
static bool
ipcp_type_is_const (enum cvalue_type type)
{
  if (type == CONST_VALUE_INT || type == CONST_VALUE_FLOAT
      || type == CONST_VALUE_INT_REF || type == CONST_VALUE_FLOAT_REF)
    return true;
  else
    return false;
}

You probably want to make this "static inline bool".


static void
ipcp_cval_compute (struct ipcp_formal *cval, struct cgraph_node *mt,
		   enum jump_func_type type, union parameter_info *info_type)
{
  if (type == UNKNOWN_IPATYPE)
  (...)

You should probably use "switch (type)" here and let the compiler figure
out what to do with it, instead of 6 times "if (type == ...)".



      if (ipcp_cval_get_cvalue_type (cval1) != CONST_VALUE_INT &&
	  ipcp_cval_get_cvalue_type (cval1) != CONST_VALUE_FLOAT &&
	  ipcp_cval_get_cvalue_type (cval1) != CONST_VALUE_INT_REF &&
	  ipcp_cval_get_cvalue_type (cval1) != CONST_VALUE_FLOAT_REF)

The &&'s should go on the new line.  IMHO you should also call this
ipcp_cval_get_cvalue_type just once for cval1 and cval2, even if it
is a static inline function.


There is this comment:
  /* ??? Handle pending sizes case. Set all parameters 
     of the method to be modified.  */
  if (DECL_UNINLINABLE (decl))

Can you explain this a bit further please, and maybe clarify the comment
a bit?


Thanks a bunch for adding the comments to ipa-prop.h, I was worried
about that ;-)  I think the pass now looks pretty clean and readable.
Could you maybe indicate how much it does in terms of transformations
on e.g. GCC itself, and maybe scores on benchmarks?

Thanks for this work.  I hope someone who can approve your work will
review it soon, so that the versioning stuff can also be submitted at
last...

Gr.
Steven


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