[PATCH] D103882: [CostModel][AArch64] Make loads/stores of <vscale x 1 x eltty> expensive.

Wed Jun 9 00:59:38 PDT 2021

dmgreen added a comment.

In D103882#2805812 <https://reviews.llvm.org/D103882#2805812>, @sdesmalen wrote:

> In D103882#2805707 <https://reviews.llvm.org/D103882#2805707>, @dmgreen wrote:
>
>> Why 9999? Why not an invalid cost? I thought that was what invalid was for, so we didn't need hacky random numbers like this.
>
> We've chosen not to conflate legalization and cost-modelling, so that the cost-model must return a valid cost when the legalization phase says that a given VF is legal to use. The benefit of that is that it guards the requirement to have a complete (albeit not necessarily accurate) cost-model. For SVE a VF of `vscale x 1` is legal in principle, meaning that we should be able to code-generate it. But because our code-generator is incomplete, we want to avoid using these VFs. I haven't added an interface to query the minimum legal VF, since I know we'll remove that interface at some point. The LV will expect all costs calculated to be Valid and will fail this assertion otherwise. So basically `return <magic number>` is just a temporary stop-gap to avoid failing that (and other) assertion failures.
>
> Does that make more sense?

I would expect the vectorizer, when encountering an invalid cost, to not pick that vector factor. Not crash. Otherwise I'm not sure I understand what an invalid cost means or is for?

I'm not sure I'm a huge fan of the proliferation of isLegal.. methods. They feel poorly defined and we have two places to return one bit of information. But I can see the advantage in ruling out large swaths of code, and stopping vectorization early when we already know it won't be valid, before having to do any expensive vplan construction. I feel we should still have invalid costs blocking vectorization at those factors though, even if it does get as far as the costmodel.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103882