[PATCH] D96287: [TTI] Change getOperandsScalarizationOverhead to take Type args

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:630-631
       if (!isa<Constant>(A) && UniqueOperands.insert(A).second) {
         auto *VecTy = dyn_cast<VectorType>(Ty);
-        if (VecTy) {
-          // If A is a vector operand, VF should be 1 or correspond to A.
-          assert((VF == 1 ||
-                  VF == cast<FixedVectorType>(VecTy)->getNumElements()) &&
-                 "Vector argument does not match VF");
-        }
-        else
-          VecTy = FixedVectorType::get(Ty, VF);
-
-        Cost += getScalarizationOverhead(VecTy, false, true);
+        if (VecTy)
+          Cost += getScalarizationOverhead(VecTy, false, true);
----------------
nit: `if(auto *VecTy = dyn_cast<VectorType>(Ty))`


================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:647
+    if (!Args.empty()) {
+      SmallVector<Type *> Tys(Args.size(), Ty);
+      Cost += getOperandsScalarizationOverhead(Args, Tys);
----------------
Is this not doing something different than it did before, i.e. it now always uses `InTy` for each of the arguments to get the cost, where it previously used the types from each of the arguments?
I expected something similar to what you did in LoopVectorize with `MaybeVectorizeType`.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp:751
             getScalarizationOverhead(cast<VectorType>(RetTy), true, false);
-      ScalarizationCost += getOperandsScalarizationOverhead(Args, RetVF);
+      ScalarizationCost += getOperandsScalarizationOverhead(Args, ICA.getArgTypes());
     }
----------------
nit: clang-format? (over 80char I think)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96287/new/

https://reviews.llvm.org/D96287