[PATCH] D133955: [AArch64][CostModel] Add costs for fixed operations when using fixed vectors over SVE

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:2164-2172
+  if (ST->hasSVE() && SrcTy.isFixedLengthVector() &&
+      DstTy.isFixedLengthVector() &&
+      SrcTy.getVectorNumElements() == DstTy.getVectorNumElements()) {
+    EVT WiderTy = SrcTy.bitsGT(DstTy) ? SrcTy : DstTy;
+    std::pair<InstructionCost, MVT> LT =
+        getTypeLegalizationCost(WiderTy.getTypeForEVT(Dst->getContext()));
+    if (LT.second.getFixedSizeInBits() > 128 ||
----------------
dtemirbulatov wrote:
> sdesmalen wrote:
> > You can remove a lot of the conditions here.
> > 
> > ```WiderTy.getFixedSizeInBits() > 128 ||
> >         (ST->forceStreamingCompatibleSVE() &&
> >          (WiderTy.is128BitVector() || WiderTy.is64BitVector()))```
> > can be replaced by:
> > ```ST->useSVEForFixedLengthVectors()```
> > 
> > You also only need to check that `SrcTy.isFixedLengthVector()`, because it's not possible to convert between scalable/fixed, similarly, the number of elements must match. This means you can change everything to:
> > 
> >   if (ST->useSVEForFixedLengthVectors() && SrcTy.isFixedLengthVector()) { 
> >     EVT WiderTy = SrcTy.bitsGT(DstTy) ? SrcTy : DstTy;
> >     std::pair<InstructionCost, MVT> LT =
> >       getTypeLegalizationCost(WiderTy.getTypeForEVT(Dst->getContext()));
> >     ...
> >     return AdjustCost(....);
> >   }
> > 
> > and do away with the nested if-condition.
> ok, but I think I still have to check for DstTy.isFixedLengthVector(). I just recently encountetred case where the source was a fixed length vector and the destanation was something like i128 type. 
Fair point, I was mostly thinking about vector-vector conversions. Bitcasts between (fixed-length) vectors and scalars should also be considered indeed.


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

https://reviews.llvm.org/D133955