[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