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

[Dinar Temirbulatov via Phabricator via llvm-commits]

dtemirbulatov marked 2 inline comments as done.
dtemirbulatov added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1935-1937
+    if (WiderTy.getFixedSizeInBits() > 128 ||
+        (ST->forceStreamingCompatibleSVE() &&
+         (WiderTy.is128BitVector() || WiderTy.is64BitVector()))) {
----------------
paulwalker-arm wrote:
> I think this can be simplified to just `((ST->useSVEForFixedLengthVectors() && WiderTy.getFixedSizeInBits() > 128) || ST->forceStreamingCompatibleSVE())` because when in streaming mode we must use SVE for all vector operations.
ah, no, I think we dont wan't to estimate 32-bit width and less vector types with SVE operations.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1938
+         (WiderTy.is128BitVector() || WiderTy.is64BitVector()))) {
+      const unsigned Multiplier = std::max<unsigned>(
+          1u, WiderTy.getFixedSizeInBits() / ST->getMinSVEVectorSizeInBits());
----------------
paulwalker-arm wrote:
> Rather than create a multiplier for the cost of one scalable vector equivalent, does creating a bigger scalable vector type not work.  That way we benefit from existing code that computes the cost of legalisation rather than second guessing it.
The legalizer does not know anything about SVE's MinSVEVectorSizeInBits and always assumes 128-bit maximum vector size as single cost if we don't have specific opertion in the table. Do we want to fix that?


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

https://reviews.llvm.org/D133955