[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

You'll want to rebase your patch on top of latest main, since D105432 <https://reviews.llvm.org/D105432> moved around the call to `getMaxVScale()` in AArch64TargetTransformInfo.cpp.



================
Comment at: clang/lib/Basic/Targets/AArch64.cpp:433
+  }
+  if (hasFeature("sve")) {
+    return std::pair<unsigned, unsigned>(0, 16);
----------------
nit: unnecessary curly braces.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:499
 
   // Add vscale attribute if appropriate.
+  Optional<std::pair<unsigned, unsigned>> VScaleRange =
----------------
nit: `vscale_range`


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1424
   ElementCount LegalVF = LT.second.getVectorElementCount();
-  Optional<unsigned> MaxNumVScale = getMaxVScale();
+  Optional<unsigned> MaxNumVScale;
+  if (I->getFunction()->hasFnAttribute(Attribute::VScaleRange)) {
----------------
This can drop the Optional now.

I also think asserting the attribute must be set is a bit of a strong requirement? Maybe we can return an Invalid instead if the attribute is not set.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106277