[FuncSpec] Don't run the solver if there's nothing to do

[SjoerdMeijer]

Summary

Even if there are no interesting functions, the SCCP solver would still run before bailing and doing nothing. Now bail earlier, avoid running the solver for nothing.

[snehasish]

llvm/lib/Transforms/IPO/FunctionSpecialization.cpp

828

Consider replacing with if(TrackedFuncs.empty()). IMO its a little easier to read.

Also I wonder if moving lines 822-831 before we define RunSCCPSolver on L793 makes it easier to read. I was confused for a bit before realizing that L793 is creating a functor. Wdyt?

llvm/test/Transforms/FunctionSpecialization/function-specialization-nothing-todo.ll

19

typo: absence

[ChuanqiXu]

LGTM with @snehasish's comment addressed.

[SjoerdMeijer]

Thanks for the review and suggestions, comments addressed.

[ChuanqiXu]

LGTM.

[snehasish]

lgtm

llvm/lib/Transforms/IPO/FunctionSpecialization.cpp

if (canTrackGlobalVariableInterprocedurally(&G))
       Solver.trackValueOfGlobalVariable(&G);
   }
 
   // Solve for constants.
   auto RunSCCPSolver = [&](auto &WorkList) {
     bool ResolvedUndefs = true;
 
     while (ResolvedUndefs) {
+      // Not running the solver unnecessary is checked in regression test
+      // nothing-to-do.ll, so if this debug message is changed, this regression
+      // test needs updating too.
       LLVM_DEBUG(dbgs() << "FnSpecialization: Running solver\n");
+
       Solver.solve();
       LLVM_DEBUG(dbgs() << "FnSpecialization: Resolving undefs\n");
       ResolvedUndefs = false;
       for (Function *F : WorkList)
         if (Solver.resolvedUndefsIn(*F))
           ResolvedUndefs = true;
     }
 
     for (auto *F : WorkList) {
       for (BasicBlock &BB : *F) {
         if (!Solver.isBlockExecutable(&BB))
           continue;
         // FIXME: The solver may make changes to the function here, so set
         // Changed, even if later function specialization does not trigger.
         for (auto &I : make_early_inc_range(BB))
           Changed |= FS.tryToReplaceWithConstant(&I);
       }
     }
   };
 
   auto &TrackedFuncs = Solver.getArgumentTrackedFunctions();
   SmallVector<Function *, 16> FuncDecls(TrackedFuncs.begin(),
                                         TrackedFuncs.end());
+
+  // No tracked functions, so nothing to do: don't run the solver and remove
+  // the ssa_copy intrinsics that may have been introduced.
+  if (!TrackedFuncs.size()) {
+    removeSSACopy(M);
+    return false;
+  }
+
 #ifndef NDEBUG
   LLVM_DEBUG(dbgs() << "FnSpecialization: Worklist fn decls:\n");
   for (auto *F : FuncDecls)
     LLVM_DEBUG(dbgs() << "FnSpecialization: *) " << F->getName() << "\n");
 #endif
 
   // Initially resolve the constants in all the argument tracked functions.
   RunSCCPSolver(FuncDecls);

llvm/test/Transforms/FunctionSpecialization/function-specialization-nothing-todo.ll

+; REQUIRES: asserts
+; RUN: opt -function-specialization -debug -S < %s 2>&1 | FileCheck %s
+
+; The purpose of this test is to check that we don't run the solver as there's
+; nothing to do here. For a test that doesn't trigger function specialisation,
+; it is intentionally 'big' because we also want to check that the ssa.copy
+; intrinsics that are introduced by the solver are cleaned up if we bail
+; early. Thus, first check the debug messages for the introduction of these
+; intrinsics:
+
+; CHECK: FnSpecialization: Analysing decl: foo
+; CHECK: Found replacement{{.*}} call i32 @llvm.ssa.copy.i32
+; CHECK: Found replacement{{.*}} call i32 @llvm.ssa.copy.i32
+
+; Then make sure the solver isn't ran:
+
+; CHECK-NOT: Running solver
+
+; Finally check the absensce and thus removal of these intrinsics:
+
+; CHECK-LABEL: @foo
+; CHECK-NOT:   call i32 @llvm.ssa.copy.i32
+
+
+@N = external dso_local global i32, align 4
+@B = external dso_local global i32*, align 8
+@A = external dso_local global i32*, align 8
+
+define dso_local i32 @foo() #0 {
+entry:
+  br label %for.cond
+
+for.cond:                                         ; preds = %for.body, %entry
+  %i.0 = phi i32 [ 0, %entry ], [ %inc, %for.body ]
+  %0 = load i32, i32* @N, align 4
+  %cmp = icmp slt i32 %i.0, %0
+  br i1 %cmp, label %for.body, label %for.cond.cleanup
+
+for.cond.cleanup:                                 ; preds = %for.cond
+  ret i32 undef
+
+for.body:                                         ; preds = %for.cond
+  %1 = load i32*, i32** @B, align 8
+  %idxprom = sext i32 %i.0 to i64
+  %arrayidx = getelementptr inbounds i32, i32* %1, i64 %idxprom
+  %2 = load i32, i32* %arrayidx, align 4
+  %3 = load i32*, i32** @A, align 8
+  %arrayidx2 = getelementptr inbounds i32, i32* %3, i64 %idxprom
+  store i32 %2, i32* %arrayidx2, align 4
+  %inc = add nsw i32 %i.0, 1
+  br label %for.cond
+}
+