[PATCH] D93030: [AArch64][SVE]Add cost model for masked gather and scatter for scalable vector.

[Caroline via Phabricator via llvm-commits]

CarolineConcatto marked 5 inline comments as done.
CarolineConcatto added a comment.

Thank you @ctetreau  and @sdesmalen  for your reviews.
I believe I've updated the patch according to your suggestions.



================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:783
+    if (MaxNumVScale.hasValue())
+      return MaxNumVScale.getValue() * VF.getKnownMinValue() * MemOpCost;
+    // TODO: Replace assert for InstructionCost
----------------
sdesmalen wrote:
> nit: It's probably still worth asking how many gather operations this requires, e.g. for `TLI->getTypeLegalizationCost(DL, DataTy);`, and then multiply:
> 
>    auto LT = TLI->getTypeLegalizationCost(DL, DataTy);
>    auto Cost = /*NumGathers*/ LT.first *
>           /*NumElementsPerGather*/ (MaxNumVScale * LT.second.getElementCount.getKnownMinValue()) *
>           MemOpCost;
> 
> Because it is multiplied, the result is currently the same, but perhaps there would be an added cost per gather instruction that is worth modelling at some point.
> 
Thank you Sander!
I've update the way we compute the cost as you've asked. 
I agree with you that is probably better to use the legalized type. 
I can see that something similar is done in getMemOpCost.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93030