[FuncSpec] Improve specializing direct constant

[duan.db]

Summary

I find that FuncSpec pass uses the option function-specialization-for-literal-constant to control whether specialize calls with direct constant.
But the control of this option may not be comprehensive enough.
An example is given as the test case.

[ChuanqiXu]

The case should be intended to be forwarded to IPSCCP pass to handle by design. Although I am odd for the design too, I thought Function Specialization should be able to cover IPSCCP once it is mature enough. @SjoerdMeijer may I ask what's the reason and plan for this restriction?

llvm/lib/Transforms/IPO/FunctionSpecialization.cpp

621–622

This might not be accurate. This doesn't just enable literal constant.

[duan.db]

Sorry, my description may not be clear.
The original case is ：

int f(){...}
int main() {
for(int i=0; i<100;i++)
  {
      f(0,i);
      f(i,13);
  }
}

ipsccp cannot optimize case like this. @ChuanqiXu

[ChuanqiXu]

Could we observe any performance/code size/compile time change on SPEC after this patch?

[SjoerdMeijer]

Just a general comment.
I am not surprised this doesn't work. The main goal of the work thus far has been to support a very specific use-case: promote indirect calls to direct calls, i.e. specialise on arguments that are function pointers. The main benefit we get out of this is inlining later on. The cost-model is tuned to this: it calculates the inlining cost/benefits.

Inlining "direct constants" is a very different use case from that. Nothing wrong with that, but again, it's just different. :-)
The main concern with this different use-case is:

• the cost model is tuned for inlining,
• and as as result, allowing specialisation for other cases is going to lead to more specialisations, resulting in increase compile-times which will be the main obstacle to get this pass enabled by default.

That's the reason we have this:

// TODO: This needs checking to see the impact on compile-times, which is why
// this is off by default for now.
static cl::opt<bool> EnableSpecializationForLiteralConstant(

Adding support for literal constants is probably the easy part. I.e., somewhere some logic needs a bit of changing, but the real question is what the cost-model is going to be? So, if we want to support literal constants, I would rather start some work on thinking what this cost-model could be. On the other hand, I also didn't want to stop any work or experimentation in this direction, so that's why I pushed for this option and it to be off by default.

llvm/lib/Transforms/IPO/FunctionSpecialization.cpp

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 (!EnableSpecializationForLiteralConstant &&
+        !Solver.getLatticeValueFor(A).isOverdefined()) {
       LLVM_DEBUG(dbgs() << "FnSpecialization: nothing to do, arg 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

llvm/test/Transforms/FunctionSpecialization/function-specialization-constant-integers1.ll

+; RUN: opt -function-specialization -function-specialization-for-literal-constant=true -force-function-specialization -S  < %s | FileCheck %s
+
+; Check that the literal constant parameter could be specialized.
+; CHECK: @f(
+; CHECK: @f.1(
+
+@.str = private unnamed_addr constant [3 x i8] c"%d\00", align 1
+
+define dso_local i32 @main(i32 %argc, i8** %argv) {
+entry:
+  br label %for.cond
+
+for.cond:                                         ; preds = %for.body, %entry
+  %i.0 = phi i32 [ 0, %entry ], [ %inc, %for.body ]
+  %cmp = icmp slt i32 %i.0, 100
+  br i1 %cmp, label %for.body, label %for.end
+
+for.body:                                         ; preds = %for.cond
+  %call = call i32 @f(i32 0, i32 %i.0)
+  %call1 = call i32 @f(i32 %i.0, i32 13)
+  %inc = add nsw i32 %i.0, 1
+  br label %for.cond
+
+for.end:                                          ; preds = %for.cond
+  ret i32 0
+}
+
+define internal i32 @f(i32 %a, i32 %x) {
+entry:
+  %cmp = icmp sgt i32 %a, 1
+  br i1 %cmp, label %if.then, label %if.else
+
+if.then:                                          ; preds = %entry
+  %mul = mul nsw i32 %a, %x
+  %call = call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @.str, i64 0, i64 0), i32 %mul)
+  br label %if.end
+
+if.else:                                          ; preds = %entry
+  %sub = sub nsw i32 %a, %x
+  %call1 = call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @.str, i64 0, i64 0), i32 %sub)
+  br label %if.end
+
+if.end:                                           ; preds = %if.else, %if.then
+  ret i32 undef
+}
+
+declare dso_local noundef i32 @printf(i8* nocapture noundef readonly, ...)
+