[PATCH] D91532: [SVE][CostModel]Extend class IntrinsicCostAttributes to use ElementCount Type

Sander de Smalen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 16 06:14:55 PST 2020


sdesmalen added a comment.

Looks like a sensible change to me.

nit: can you please add `NFC:` in front of the title, like: `NFC: Extend class ...`, that way people can easily see the patch is NFC and shouldn't cause any side-effects.
nit: the title has the tag `[SVE]`, but there is nothing specific to SVE in this patch, so you can remove it.



================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:1151
 
-    unsigned VF = ICA.getVectorFactor();
+    unsigned VF = ICA.getVectorFactor().getKnownMinValue();
     unsigned RetVF =
----------------
Can you also change `unsigned VF` and `unsigned RetVF` to be of type `ElementCount`?
>From what I can see, the changes required for that are quite mechanical, as long as you 'stop' at `getOperandsScalarizationOverhead`, because that seems like an interface change for another patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91532



More information about the llvm-commits mailing list