[llvm] [FnSpecialization] Only accept codesize savings if strictly greater than the minimum amount (PR #164867)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Ryan Buchner (bababuck)

<details>
<summary>Changes</summary>

When `--funcspec-min-function-size` is set to a small value, any small function (provided passed a constant) will be scored favorably in terms of code size savings due to the rounding in integer division in the following statement:
```
auto IsProfitable = [&]() -> bool {
  ...
  if (CodeSizeSavings < MinCodeSizeSavings * FuncSize / 100)
    return false;
  ...
};
```
When MinCodeSizeSavings is set by default to 20, so if `FuncSize < 5` all allowed by the knob, then `MinCodeSizeSavings * FuncSize / 100` evaluates to 0. As a result, a CodeSizeSavings of `0` will pass this check and be possibly considered profitable.

I don't think this is generally an issue in the wild since `--funcspec-min-function-size` is not usually set to small values (it defaults to 500), but might be on inline cases.

Prior to this change, `unprofitable_spec` specializes (with `--funcspec-min-function-size=0`) despite not having any savings.
```
define i32 @<!-- -->main(i32 %y) {
  %spec = call i32 @<!-- -->unprofitable_spec(i32 1, i32 %y)
  ret i32 %spec
}

define i32 @<!-- -->unprofitable_spec(i32 %x, i32 %y) {
  %add = add i32 %x, %y
  ret i32 %add
}
```

---
Full diff: https://github.com/llvm/llvm-project/pull/164867.diff


5 Files Affected:

- (modified) llvm/lib/Transforms/IPO/FunctionSpecialization.cpp (+1-1) 
- (modified) llvm/test/Transforms/FunctionSpecialization/dead-gv-load.ll (+2-1) 
- (modified) llvm/test/Transforms/FunctionSpecialization/maxgrowth.ll (+1-1) 
- (added) llvm/test/Transforms/FunctionSpecialization/min-codesize-savings.ll (+28) 
- (modified) llvm/test/Transforms/FunctionSpecialization/recursive-penalty.ll (+2-1) 


``````````diff
diff --git a/llvm/lib/Transforms/IPO/FunctionSpecialization.cpp b/llvm/lib/Transforms/IPO/FunctionSpecialization.cpp
index 150a2dc5d48e2..6d4b2fb7e0065 100644
--- a/llvm/lib/Transforms/IPO/FunctionSpecialization.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionSpecialization.cpp
@@ -995,7 +995,7 @@ bool FunctionSpecializer::findSpecializations(Function *F, unsigned FuncSize,
                    << (CodeSizeSavings * 100 / FuncSize) << "%)}\n");
 
         // Minimum codesize savings.
-        if (CodeSizeSavings < MinCodeSizeSavings * FuncSize / 100)
+        if (CodeSizeSavings <= MinCodeSizeSavings * FuncSize / 100)
           return false;
 
         // Lazily compute the Latency, to avoid unnecessarily computing BFI.
diff --git a/llvm/test/Transforms/FunctionSpecialization/dead-gv-load.ll b/llvm/test/Transforms/FunctionSpecialization/dead-gv-load.ll
index 134a79d349035..45e41bd1bdce9 100644
--- a/llvm/test/Transforms/FunctionSpecialization/dead-gv-load.ll
+++ b/llvm/test/Transforms/FunctionSpecialization/dead-gv-load.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
-; RUN: opt -passes=ipsccp  --funcspec-min-function-size=1 -S < %s | FileCheck %s
+; RUN: opt -passes=ipsccp  --funcspec-min-function-size=1 \
+; RUN: -funcspec-min-codesize-savings=1 -S < %s | FileCheck %s
 
 @gv = internal global ptr null
 
diff --git a/llvm/test/Transforms/FunctionSpecialization/maxgrowth.ll b/llvm/test/Transforms/FunctionSpecialization/maxgrowth.ll
index 82d1f7ae4a6e1..c07948489dd5e 100644
--- a/llvm/test/Transforms/FunctionSpecialization/maxgrowth.ll
+++ b/llvm/test/Transforms/FunctionSpecialization/maxgrowth.ll
@@ -1,7 +1,7 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --include-generated-funcs --version 5
 ; RUN: opt -passes="ipsccp<func-spec>" -funcspec-min-function-size=1       \
 ; RUN:                                 -funcspec-for-literal-constant=true \
-; RUN:                                 -funcspec-min-codesize-savings=50   \
+; RUN:                                 -funcspec-min-codesize-savings=1    \
 ; RUN:                                 -funcspec-min-latency-savings=50    \
 ; RUN:                                 -funcspec-max-codesize-growth=1     \
 ; RUN:                                 -S < %s | FileCheck %s
diff --git a/llvm/test/Transforms/FunctionSpecialization/min-codesize-savings.ll b/llvm/test/Transforms/FunctionSpecialization/min-codesize-savings.ll
new file mode 100644
index 0000000000000..4547e0e009852
--- /dev/null
+++ b/llvm/test/Transforms/FunctionSpecialization/min-codesize-savings.ll
@@ -0,0 +1,28 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
+; RUN: opt -passes=ipsccp  --funcspec-min-function-size=0 -S < %s | FileCheck %s
+
+; Call to unprofitable_spec should not specialize since no folding can occur for the add
+; instruction as only 1 of the values is know. The score for this specialization will be 0.
+define i32 @main(i32 %y) {
+; CHECK-LABEL: define i32 @main(
+; CHECK-SAME: i32 [[Y:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[SPEC:%.*]] = call i32 @unprofitable_spec(i32 1, i32 [[Y]])
+; CHECK-NEXT:    ret i32 [[SPEC]]
+;
+entry:
+  %spec = call i32 @unprofitable_spec(i32 1, i32 %y)
+  ret i32 %spec
+}
+
+define i32 @unprofitable_spec(i32 %x, i32 %y) {
+; CHECK-LABEL: define i32 @unprofitable_spec(
+; CHECK-SAME: i32 [[X:%.*]], i32 [[Y:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[ADD:%.*]] = add i32 [[X]], [[Y]]
+; CHECK-NEXT:    ret i32 [[ADD]]
+;
+entry:
+  %add = add i32 %x, %y
+  ret i32 %add
+}
diff --git a/llvm/test/Transforms/FunctionSpecialization/recursive-penalty.ll b/llvm/test/Transforms/FunctionSpecialization/recursive-penalty.ll
index fc17387dec94d..409e7b870355d 100644
--- a/llvm/test/Transforms/FunctionSpecialization/recursive-penalty.ll
+++ b/llvm/test/Transforms/FunctionSpecialization/recursive-penalty.ll
@@ -1,7 +1,8 @@
 ; REQUIRES: asserts
 ; RUN: opt -passes="ipsccp<func-spec>,inline,instcombine,simplifycfg" -S \
 ; RUN:     -funcspec-min-function-size=23 -funcspec-max-iters=100 \
-; RUN:     -debug-only=function-specialization < %s 2>&1 | FileCheck %s
+; RUN:     -debug-only=function-specialization -funcspec-min-codesize-savings=1 \
+; RUN:     < %s 2>&1 | FileCheck %s
 
 ; Make sure the number of specializations created are not
 ; linear to the number of iterations (funcspec-max-iters).

``````````

</details>


https://github.com/llvm/llvm-project/pull/164867