[VectorCombine][X86] old-cost model overestimates `load + insertelement poison` patterns on x86
- Dominant language
- LLVM
- Stars
- 40.5k
- Forks
- 18.7k
- PR merge metrics
- PR metrics pending
Description
### Summary
While investigating the `NeedCast` path in `VectorCombine::vectorizeLoadInsert`, I noticed that the old-cost calculation can overestimate the cost of the original IR pattern:
```llvm
%s = load i32, ptr %gep, align 1
%r = insertelement <4 x i32> poison, i32 %s, i64 0
```
On x86, this pattern can lower to a single scalar memory load into a vector register, for example:
```asm
vmovss xmm0, dword ptr [rdi + 5]
ret
```
However, the current VectorCombine cost model charges both the scalar load cost and a scalarization/insert overhead:
```text
OldLoadCost: 1
OldScalarizationCost: 1
OldCost: 2
```
This makes the original form look more expensive than it actually is.
### Reproducer
```llvm
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
define <4 x i32> @gep05_bitcast_load_i32_from_v8i16_insert_v4i32(ptr align 1 dereferenceable(16) %p) {
%gep = getelementptr inbounds <8 x i16>, ptr %p, i64 0, i64 5
%s = load i32, ptr %gep, align 1
%r = insertelement <4 x i32> poison, i32 %s, i64 0
ret <4 x i32> %r
}
```
A corresponding transformed form is:
```llvm
define <4 x i32> @candidate(ptr align 1 dereferenceable(16) %p) {
%wide = load <8 x i16>, ptr %p, align 1
%shuf = shufflevector <8 x i16> %wide, <8 x i16> poison,
<8 x i32>
%r = bitcast <8 x i16> %shuf to <4 x i32>
ret <4 x i32> %r
}
```
On x86 with AVX, the original form lowers to:
```asm
vmovss xmm0, dword ptr [rdi + 5]
ret
```
The transformed form lowers to something like:
```asm
vmovdqu xmm0, xmmword ptr [rdi]
vpsrldq xmm0, xmm0, 10
ret
```
So the original code is effectively one instruction, while the transformed code is a vector load plus a byte shift.
### Cost model observation
I added debug logging around the old-cost calculation in `VectorCombine::vectorizeLoadInsert`:
```text
VC load-insert old cost:
LoadTy: i32
VecTy: <4 x i32>
DemandedElts: 1
HasExtract: 0
OldLoadCost: 1
OldScalarizationCost: 1
OldCost: 2
VC load-insert cost:
NeedCast: 1
LoadTy: i32
MinVecTy: <8 x i16>
ResultTy: <4 x i32>
OldCost: 2 vs NewCost: 2
```
The old cost is computed as:
```cpp
OldCost = TTI.getMemoryOpCost(... load i32 ...);
OldCost += TTI.getScalarizationOverhead(VecTy, DemandedElts,
/*Insert=*/true,
HasExtract,
CostKind);
```
I also checked whether passing `ForPoisonSrc=true` to `getScalarizationOverhead()` fixes the estimate, but the scalarization overhead still does not drop to zero for this case.
### Problem
The issue appears to be that `getScalarizationOverhead()` models the cost of building a vector from scalar values. That is reasonable for a pattern like:
```llvm
%x = some scalar value
%r = insertelement <4 x i32> poison, i32 %x, i64 0
```
However, the old form in `vectorizeLoadInsert` is more specific:
```llvm
%s = load i32, ptr %gep, align 1
%r = insertelement <4 x i32> poison, i32 %s, i64 0
```
On x86, this can be selected as a single memory load directly into the vector register. In other words, the scalar load and the lane-0 insertion are not two separate costs in the final code.
The current cost model therefore overestimates the old form:
```text
model:
scalar load cost + insert/scalarization overhead = 2
actual x86 lowering:
scalar memory load into vector register = 1
```
This can make VectorCombine accept transforms that are not actually profitable.
### Expected behavior
For the specific old pattern:
```llvm
insertelement poison, (one-use load T), 0
```
the cost model should not blindly add a scalarization/insert overhead if the target can lower the original form as a single memory load into a vector register.
### Possible directions
There are a few possible ways to address this:
1. Teach `getScalarizationOverhead()` / target TTI to account for this kind of memory-source insertion pattern.
2. Extend the cost query so the caller can express that the inserted scalar value comes directly from a load.
3. Handle this specifically at the `VectorCombine::vectorizeLoadInsert` call site, since that transform already knows it is comparing a `load + insertelement` compound pattern.
I am not sure which direction is preferred. The important point is that the current use of `getScalarizationOverhead()` in this call site does not accurately model the original x86 lowering for `load + insertelement poison, lane 0`.
Contributor guide
Assessment
This issue has not been assessed yet.