[PATCH] D145819: [FuncSpec] Increase the maximum number of times the specializer can run.

[Alexandros Lamprineas via Phabricator via llvm-commits]

labrinea added a comment.

I am planning changes to D145819 <https://reviews.llvm.org/D145819>. It makes sense to run promoteConstantStackValues per function and not across the entire module, at least until specialization on literal constants gets enabled. The reason is that there can't be new opportunities in consecutive iterations of run() unless we are specializing a call which uses a return value from a specialization created in the previous iteration, or a call which resides in the body of a specialization. It seems to me that promoteConstantStackValues was specifically written for recursive functions, so perhaprs we could limit it there like:

  @@ -476,17 +464,37 @@ bool FunctionSpecializer::run() {
       if (!isCandidateFunction(&F))
         continue;
  
  -    Cost SpecCost = getSpecializationCost(&F);
  -    if (!SpecCost.isValid()) {
  -      LLVM_DEBUG(dbgs() << "FnSpecialization: Invalid specialization cost for "
  -                        << F.getName() << "\n");
  -      continue;
  +    auto [It, Inserted] = FunctionMetrics.try_emplace(&F);
  +    CodeMetrics &Metrics = It->second;
  +    //Analyze the function.
  +    if (Inserted) {
  +      SmallPtrSet<const Value *, 32> EphValues;
  +      CodeMetrics::collectEphemeralValues(&F, &GetAC(F), EphValues);
  +      for (BasicBlock &BB : F)
  +        Metrics.analyzeBasicBlock(&BB, GetTTI(F), EphValues);
       }
  
  +    // If the code metrics reveal that we shouldn't duplicate the function,
  +    // or if the code size implies that this function is easy to get inlined,
  +    // then we shouldn't specialize it.
  +    if (Metrics.notDuplicatable || !Metrics.NumInsts.isValid() ||
  +        (!ForceSpecialization && !F.hasFnAttribute(Attribute::NoInline) &&
  +         Metrics.NumInsts < MinFunctionSize))
  +      continue;
  +
  +    // TODO: For now only consider recursive functions when running multiple
  +    // times. This should change if specialization on literal constants gets
  +    // enabled.
  +    if (!Inserted && !Metrics.isRecursive && !SpecializeLiteralConstant)
  +      continue;
  +
       LLVM_DEBUG(dbgs() << "FnSpecialization: Specialization cost for "
  -                      << F.getName() << " is " << SpecCost << "\n");
  +                      << F.getName() << " is " << Metrics.NumInsts << "\n");
  +
  +    if (Metrics.isRecursive)
  +      promoteConstantStackValues(&F);
  
  -    if (!findSpecializations(&F, SpecCost, AllSpecs, SM)) {
  +    if (!findSpecializations(&F, Metrics.NumInsts, AllSpecs, SM)) {
         LLVM_DEBUG(
             dbgs() << "FnSpecialization: No possible specializations found for "
                    << F.getName() << "\n");


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145819/new/

https://reviews.llvm.org/D145819