InlineCost::get() relies on asserts to reject sentinel costs, causing debug/release inconsistency with inline-cost testing attributes
- Dominant language
- LLVM
- Stars
- 40.5k
- Forks
- 18.7k
- PR merge metrics
- PR metrics pending
Description
Hello,
I found a small robustness issue in `llvm/lib/Analysis/InlineCost.cpp` / `llvm/include/llvm/Analysis/InlineCost.h`.
`InlineCost` uses `INT_MIN` and `INT_MAX` as sentinel values for `AlwaysInline` and `NeverInline`:
- `AlwaysInlineCost = INT_MIN`
- `NeverInlineCost = INT_MAX`
and `InlineCost::get(int Cost, int Threshold, ...)` currently only protects against those values with asserts:
```cpp
assert(Cost > AlwaysInlineCost && "Cost crosses sentinel value");
assert(Cost < NeverInlineCost && "Cost crosses sentinel value");
```
However, InlineCost.cpp has a few test/debug-oriented string attributes that can directly override or manipulate the computed cost/threshold, for example:
```cpp
"function-inline-cost"
"function-inline-threshold"
"function-inline-cost-multiplier"
"call-inline-cost"
"call-threshold-bonus"
```
These attributes were introduced to make inliner testing easier, but they also make it possible to drive the variable inline cost into sentinel territory. For example, a test can set:
```cpp
// test.ll
define void @callee() {
entry:
ret void
}
define void @caller() {
entry:
call void @callee()
"function-inline-cost"="3"
"function-inline-cost-multiplier"="1073741824"
"function-inline-threshold"="100"
ret void
}
```
This code was originally supposed to prevent inlining because `cost > threshold`, but it ended up being inlined.
```cpp
// out.ll
; ModuleID = 'test.ll'
source_filename = "test.ll"
define void @callee() {
entry:
ret void
}
define void @caller() {
entry:
ret void
}
```
So the current behavior differs between debug and release builds in a way that is probably unintended.
I realize these attributes are mainly intended for testing/debugging, so this is not a high-severity issue. Still, it seems worth fixing because the change can be very small and would make behavior more robust and consistent.
A minimal fix would be to make InlineCost::get() clamp variable costs away from sentinel values instead of relying solely on asserts, e.g. by mapping:
```cpp
Cost <= INT_MIN to INT_MIN + 1
Cost >= INT_MAX to INT_MAX - 1
```
That would preserve the special meaning of the sentinel values while avoiding debug/release divergence.
If this sounds reasonable, I can send a patch.
Thanks!
Contributor guide
Assessment
This issue has not been assessed yet.