[llvm] [SLP][REVEC] Remove assert because canConvertToMinOrMaxIntrinsic can support vector type. (PR #114946)
Han-Kuan Chen via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 5 07:49:49 PST 2024
================
@@ -10777,7 +10777,6 @@ BoUpSLP::getEntryCost(const TreeEntry *E, ArrayRef<Value *> VectorizedVals,
// If the selects are the only uses of the compares, they will be
// dead and we can adjust the cost by removing their cost.
if (VI && SelectOnly) {
- assert(!Ty->isVectorTy() && "Expected only for scalar type.");
----------------
HanKuanChen wrote:
I think the reason we use assert here is we don't want `GetMinMaxCost` accidentally remove the cost of `CmpInst` when the cost of vectorized code is estimated.
```
case TreeEntry::MinMax: {
auto GetScalarCost = [&](unsigned Idx) {
return GetMinMaxCost(OrigScalarTy);
};
auto GetVectorCost = [&](InstructionCost CommonCost) {
InstructionCost VecCost = GetMinMaxCost(VecTy);
return VecCost + CommonCost;
};
return GetCostDiff(GetScalarCost, GetVectorCost);
}
```
But `GetVectorCost` does not pass `VI` to `GetMinMaxCost`, so the assert will never be triggered.
Add `SLPReVec` here is weird because they mean different thing. It will make `GetVectorCost` accidentally minus the cost of `CmpInst` when `REVEC` is enabled. But it violates the original purpose of the assert.
https://github.com/llvm/llvm-project/pull/114946
More information about the llvm-commits
mailing list