[PATCH] D96253: [BasicTTIImpl] Fix getCastInstrCost for scalable vectors by querying for ElementCount.

[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!



================
Comment at: llvm/test/Analysis/CostModel/AArch64/cast.ll:711
 ; CHECK-NEXT:  Cost Model: Found an estimated cost of 1 for instruction: %loadv4i32 = load <4 x i32>, <4 x i32>* undef
+; CHECK-NEXT:  Cost Model: Found an estimated cost of 1 for instruction: %loadnxv2i32 = load <vscale x 2 x i32>, <vscale x 2 x i32>* undef
+; CHECK-NEXT:  Cost Model: Found an estimated cost of 1 for instruction: %loadnxv4i32 = load <vscale x 4 x i32>, <vscale x 4 x i32>* undef
----------------
sdesmalen wrote:
> david-arm wrote:
> > Hi @sdesmalen, given you've added support in the cost function for scalable vectors of the type <vscale x 1 x ElTy> perhaps it's worth adding a test for that too? i.e.
> > 
> > ```%loadnxv1i32 = load <vscale x 1 x i32>, <vscale x 1 x i32>* undef```
> > 
> I could, but I'm not sure how helpful that is, because we can't currently vectorize <vscale x 1 x <eltty>>. For this the cost //should// return `Invalid`, but it just returns some value now. This is probably something to fix in a follow-up patch at some point. It also wouldn't help test the code-change in this patch.
OK that's fine. I think it'd be good to fix getCastInstrCost to deal with the <vscale x 1 x> case properly then at some point, since at the moment it just lets them through. I guess this is a general problem for a lot of our cost model right now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96253