[FuncSpec] Enable function specialize for a constant range of arguments

[Joe]

Summary

This will consider function specialization for candidate constants in a
constant range where the range represents all the possible arguments.

Currently, this will only happen when
-function-specialization-for-literal-constant is supplied.

Arguments from calls that are constant ranges themselves are still
unsupported.

I have a feeling I might be missing something here. Is there a reason why this wasn't already the case?

Tests affected (function-specialization.NumFuncSpecialized)

test	before	after
MultiSource/Applications/SPASS/SPASS.test	NaN	11.00
MultiSource/Applications/JM/ldecod/ldecod.test	NaN	3.00
MultiSource/Applications/oggenc/oggenc.test	2.00	3.00

I tried measuring performance (Cycles/Instructions) with perf on x86, but there was too much noise in the results to gather anything meaningful.

Looking into the specialisations themselves, it often looks good. For some it introduces inlining opportunities. However, whether or not these were objectively good changes should be handled by the cost analysis. This change simply considers more opportunities.

[labrinea]

My understanding is that function specialization of such constant arguments is controlled by the option -function-specialization-for-literal-constant. That is disabled by default as it increases compilation times. I measured how it affects instruction count using perf when compiling the llvm test suite. It was generally neutral except for the benchmark SPASS which was regressed by 4%.

[Joe]

In D120908#3358301, @labrinea wrote:

My understanding is that function specialization of such constant arguments is controlled by the option -function-specialization-for-literal-constant. That is disabled by default as it increases compilation times. I measured how it affects instruction count using perf when compiling the llvm test suite. It was generally neutral except for the benchmark SPASS which was regressed by 4%.

Right I see. EnableSpecializationForLiteralConstant was an oversight for me. So what if EnableSpecializationForLiteralConstant is enabled, yet a function only takes literal constants, like in the unit test supplied. isArgumentInteresting would exit early due to the isOverdefined check. Should this be changed as is in this revision, but the change to getPossibleConstants be reverted?

[ChuanqiXu]

Personally, I feel the design principle of current FuncSpec is to leave opportunities to IPSCCP if possible. I guess the main concern may be compilation time.
(Although I feel FuncSpec could cover IPSCCP.)

llvm/lib/Transforms/IPO/FunctionSpecialization.cpp

683–690

My personal feeling is that FuncSpec might be able to replace IPSCCP completely if FuncSpec is ready enough. I tried to remove the whole check here in downstream and didn't see any regression in performance. (I didn't measure compile time though).

[Joe]

Updated diff. Now a very small change.

llvm/lib/Transforms/IPO/FunctionSpecialization.cpp

private:
   /// argument.
   bool isArgumentInteresting(Argument *A, ConstList &Constants,
                              bool &IsPartial) {
     // For now, don't attempt to specialize functions based on the values of
     // composite types.
     if (!A->getType()->isSingleValueType() || A->user_empty())
       return false;
 
-    // If the argument isn't overdefined, there's nothing to do. It should
-    // already be constant.
-    if (!Solver.getLatticeValueFor(A).isOverdefined()) {
+    // If the argument is Constant, there's nothing to do. IPSCCP will
+    // propagate the argument.
+    if (isConstant(Solver.getLatticeValueFor(A))) {
       LLVM_DEBUG(dbgs() << "FnSpecialization: Nothing to do, argument "
                         << A->getNameOrAsOperand()
                         << " is already constant?\n");
       return false;
     }
 
     // Collect the constant values that the argument can take on. If the
     // argument can't take on any constant values, we aren't going to
     // specialize the function. While it's possible to specialize the function
     // based on non-constant arguments, there's likely not much benefit to
     // constant propagation in doing so.
     //
     // TODO 1: currently it won't specialize if there are over the threshold of
     // calls using the same argument, e.g foo(a) x 4 and foo(b) x 1, but it
     // might be beneficial to take the occurrences into account in the cost
     // model, so we would need to find the unique constants.
     //
-    // TODO 2: this currently does not support constants, i.e. integer ranges.
+    // TODO 2: this currently does not support constant range arguments.
     //
     IsPartial = !getPossibleConstants(A, Constants);
     LLVM_DEBUG(dbgs() << "FnSpecialization: Found interesting argument "
                       << A->getNameOrAsOperand() << "\n");
     return true;
   }
 
   /// Collect in \p Constants all the constant values that argument \p A can

llvm/test/Transforms/FunctionSpecialization/function-specialization6.ll

+; RUN: opt -function-specialization -function-specialization-for-literal-constant -force-function-specialization -S < %s | FileCheck %s
+
+define i64 @main(i64 %x, i1 %flag) {
+;
+; CHECK-LABEL: @main(i64 %x, i1 %flag) {
+; CHECK:      entry:
+; CHECK-NEXT:   br i1 %flag, label %three, label %four
+; CHECK:      three:
+; CHECK-NEXT:   [[TMP0:%.+]] = call i64 @compute.1(i64 3, i64 (i64)* @plus)
+; CHECK-NEXT:   br label %merge
+; CHECK:      four:
+; CHECK-NEXT:   [[TMP1:%.+]] = call i64 @compute.2(i64 4, i64 (i64)* @plus)
+; CHECK-NEXT:   br label %merge
+; CHECK:      merge:
+; CHECK-NEXT:   [[TMP2:%.+]] = phi i64 [ [[TMP0]], %three ], [ [[TMP1]], %four ]
+; CHECK-NEXT:   ret i64 [[TMP2]]
+; CHECK-NEXT: }
+;
+entry:
+  br i1 %flag, label %three, label %four
+
+three:
+  %tmp0 = call i64 @compute(i64 3, i64 (i64)* @plus)
+  br label %merge
+
+four:
+  %tmp1 = call i64 @compute(i64 4, i64 (i64)* @plus)
+  br label %merge
+
+merge:
+  %tmp2 = phi i64 [ %tmp0, %three ], [ %tmp1, %four]
+  ret i64 %tmp2
+}
+
+; CHECK-NOT: define internal i64 @compute(
+;
+; CHECK-LABEL: define internal i64 @compute.1(i64 %x, i64 (i64)* %binop) {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TMP0:%.+]] = call i64 @plus(i64 3)
+; CHECK-NEXT:    ret i64 [[TMP0]]
+; CHECK-NEXT:  }
+;
+; CHECK-LABEL: define internal i64 @compute.2(i64 %x, i64 (i64)* %binop) {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TMP0:%.+]] = call i64 @plus(i64 4)
+; CHECK-NEXT:    ret i64 [[TMP0]]
+; CHECK-NEXT:  }
+;
+define internal i64 @compute(i64 %x, i64 (i64)* %binop) {
+entry:
+  %tmp0 = call i64 %binop(i64 %x)
+  ret i64 %tmp0
+}
+
+define internal i64 @plus(i64 %x) {
+entry:
+  %tmp0 = add i64 %x, 1
+  ret i64 %tmp0
+}