[llvm-commits] [llvm] r55734 - in /llvm/trunk: include/llvm/LinkAllPasses.h include/llvm/Transforms/IPO.h lib/Transforms/IPO/PartialSpecialization.cpp

Duncan Sands baldrick at free.fr
Thu Sep 4 01:19:42 PDT 2008


Hi,

> +// The initial heuristic favors constant arguments that used in control flow.

that used -> that are used

> +//Call must be used at least occasionally
> +static const int CallsMin = 5;
> +//Must have 10% of calls having the same constant to specialize on
> +static const double ConstValPercent = .1;

No space after //; missing full stop at end of comments.
Likewise below.

> +  for (Module::iterator I = M.begin(); I != M.end(); ++I) {
> +    Function &F = *I;
> +    if (!F.isDeclaration()) {

how about:
  if (F.isDeclaration())
    continue;
to avoid a level of indentation.

> +      std::vector<int> interestingArgs;

Maybe a SmallVector is better, since there won't be that many
arguments usually.

> +      //If there are multiple intersting Arguments, then those will be found

intersting -> interesting

> +            if ( total > ii->second  && ii->first &&

Space after opening "(" (likewise for closing).  Two spaces
before first &&.

> +                 ii->second > total * ConstValPercent ) {

If a function is called less than 10 times, then every call to it
which has a constant argument somewhere will result in cloning.  Is
that right?  Isn't that going to result in fat bitcode?

> +/// scanForInterest - This function decides which arguments would be worth
> +///                    specializing on.

Usually the second comment line is not lined up like this.

> +      if (isa<CmpInst>(ui)) {

This is the bit about "used in control flow" I suppose.
Presumably this will be made more sophisticated later.

> +    if (CallInst* CI = dyn_cast<CallInst>(ii))

How about handling invokes too?

> +  bool hasInd = false;

What does "hasInd" mean?  Ind -> Indirect?  Is it really
useful to increment total if hasInd is true?

Ciao,

Duncan.



More information about the llvm-commits mailing list