Skip to content

Commit bec9ac2

Browse files
authored
[llvm] Lower latency bonus threshold in function specialization. (#143954)
Related to #143219. Function specialization does not kick in if flang sets `noalias` attributes on the function arguments of `digits_2`, because PRE optimizes several `srem` instructions and other memory accesses from the inner loops causing the latency bonus to be lower than the current 40% threshold. While looking at this, I did not really get why we compute the latency bonus as a ratio of the latency of the "eliminated" instructions and the code-size of the whole function. It did not make much sense to me. I tried computing the total latency as a sum of latencies of the instructions that belong to non-dead code (including the instructions that would be executed had they not been "eliminated" due to the constant propagation). This total latency should identify the total cost of executing the function with the given argument being dynamically equal to the tried constant value. Then the latency bonus would be computed as the ratio between the latency of the "eliminated" instructions and the total latency. Unfortunately, this did not given me a good heuristics either. The bonus was close to 0% on some targets, and as big as 3-5% on other targets. This does match very well with the performance gain achieved by function specialization for exchange2, so it seemd like another artificial heuristic not better than the current one. It seems that GCC uses a set of different heuristics for function specialization, but I am not an expert here and I cannot say if we can match them in LLVM. With all that said, I decided to try to lower the threshold to avoid the regression and be able to re-enable the generally good change for `noalias` attribute. With this patch, I was able to reduce the effect of `noalias`, so that `-force-no-alias=true` is only ~10% slower than `-force-no-alias=false` code on neoverse-v1 and neoverse-v2. On neoverse-n1, `-force-no-alias=true` is >2x faster than `-force-no-alias=false` regardless of this patch. This threshold has been changed before also due to improved alias information: 2fb51fb#diff-066363256b7b4164e66b28a3028b2cb9e405c9136241baa33db76ebd2edb87cd Please let me know what testing I should run to make sure this change is safe. As I understand, it may affect the compilation time performance, and I will appreciate it if someone points out which benchmarks need to be checked before merging this.
1 parent 3800a83 commit bec9ac2

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

llvm/lib/Transforms/IPO/FunctionSpecialization.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ static cl::opt<unsigned> MinCodeSizeSavings(
7171
"much percent of the original function size"));
7272

7373
static cl::opt<unsigned> MinLatencySavings(
74-
"funcspec-min-latency-savings", cl::init(40), cl::Hidden,
74+
"funcspec-min-latency-savings", cl::init(20), cl::Hidden,
7575
cl::desc("Reject specializations whose latency savings are less than this "
7676
"much percent of the original function size"));
7777

0 commit comments

Comments
 (0)