[PATCH] D132477: Improve cost model for some 128-bit vector operations that use SVE

[David Sherwood via Phabricator via llvm-commits]

david-arm accepted this revision.
david-arm added a comment.
This revision is now accepted and ready to land.

LGTM with nits addressed! Thanks for making all the changes @hassnaa-arm.



================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:2098
+    // the SVE mul instruction, which has a lower cost.
+    if (LT.second == MVT::v2i64 && (ST->hasSVE()))
+      return LT.first;
----------------
nit: You don't need the extra brackets `()` around `ST->hasSVE()`


================
Comment at: llvm/test/Analysis/CostModel/AArch64/sve-fixed-length.ll:124
+
+; Assuming base_cost = 2 * ((vec_len-1/VBITS)+1)
+; Assuming legalization_cost = (vec_len-1/VBITS)+1
----------------
nit: I think this should be the same as the sdiv case, i.e. just

  ; Assuming base_cost = 2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132477