[PATCH] D146839: [TLI][AArch64] Extend SLEEF vectorized functions mapping with VLA functions

Sander de Smalen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 28 05:36:36 PDT 2023


sdesmalen added inline comments.


================
Comment at: llvm/include/llvm/Analysis/VecFuncs.def:21
 #define FIXED(NL) ElementCount::getFixed(NL)
 #define SCALABLE(NL) ElementCount::getScalable(NL)
+#define NOMASK 0
----------------
danielkiss wrote:
> I won't introduce a new argument here, since `MASKED` is just indicate this interface is scalable so far.
> Can we make the declarations less verbose?
> e.g.: 
> #define FIXED(NL) ElementCount::getFixed(NL), false
> #define SCALABLE(NL) ElementCount::getScalable(NL), true 
Sorry I missed this comment yesterday, but I'm not sure I agree with the suggestion to let SCALABLE imply MASKED. The two concepts are orthogonal, they just happen to have the same value for the set of vector function mappings added in this patch.  It's perfectly possible to have an unmasked scalable interface and we may want to add those in the future.

The cost of adding the mask separately seemed quite low, is there another reason you prefer not to add this now?


================
Comment at: llvm/lib/CodeGen/ReplaceWithVeclib.cpp:159
   const std::string TLIName =
-      std::string(TLI.getVectorizedFunction(ScalarName, VF));
+      std::string(TLI.getVectorizedFunction(ScalarName, VF, false));
 
----------------
Can you make `false` a default option for this argument, so that you don't explicitly have to pass it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146839



More information about the llvm-commits mailing list