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

Dave Green via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 11 04:03:35 PST 2021


dmgreen added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:647
+    if (!Args.empty()) {
+      SmallVector<Type *> Tys(Args.size(), Ty);
+      Cost += getOperandsScalarizationOverhead(Args, Tys);
----------------
sdesmalen wrote:
> dmgreen wrote:
> > sdesmalen wrote:
> > > 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`.
> > Yeah. This version of getScalarizationOverhead is only called from getArithmeticInstrCost (or ARMTTIImpl::getArithmeticInstrCost etc). So the arguments/return value are all likely to all be the same value, and this is making that assumption.  It is probably at least good enough as a heuristic, when compared to the else below. I've added a comment too.
> If this code assumes all arguments are of the same type, can you add an `assert` which checks this?
Unfortunately that isn't something that is always true. The type of Args may be incorrect, even the scalar type. The vectorizer can do bitwidth narrowing at the same time as vectorization, in which case the Ty will be the most accurate type available. If the original Arg type was an i32, but the vectorizer is vectorizing with it as an i8 to maximize the vector factor, a vXi8 is the correct type to use. That will be Ty for all the uses of this function.

I could try to move this array creation to the callers of the function. That would involve duplicating this bit of code in a few places, but after e771614bae0a05 removed the only other use of it, it should ensure these types are correct from the outside.


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

https://reviews.llvm.org/D96287



More information about the llvm-commits mailing list