[PATCH] D121109: [AArch64] Add cost model for llvm.get.active.lane.mask intrinsic

Tue Mar 8 05:00:03 PST 2022

dmgreen added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:234
+    // This should be a single instruction.
+    if (!TLI->shouldExpandGetActiveLaneMask(ResVT, ArgType))
+      return 1;
----------------
david-arm wrote:
> dmgreen wrote:
> > sdesmalen wrote:
> > > Can the default case (i.e. when `shouldExpandGetActiveLaneMask = true`) be added to BasicTTIImpl?
> > > Then in the target's TTI, you only need to handle the case where the intrinsic is not expanded.
> > Does this need to be here? Or could it be part of the target independent code - assuming the cost is 1 if it is not expanded?
> That's a good idea. One thing that makes me uncertain of that is that I can't call the usual `TLI->getTypeLegalizationCost` function to get the value type because for NEON the legalised type is transformed from v4i1 -> v4i32, etc. This then changes the behaviour of `shouldExpandGetActiveLaneMask`. That's why I only call `TLI->getValueType` here. If there is precedence for doing this in BasicTTIImpl then I'm happy to move it there.
I think that sounds fine, if it makes more sense to do that. Some of the existing places already do so - like the cost of memory ops.

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

https://reviews.llvm.org/D121109