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

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer 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 think I understand that, but I am afraid I am still with Dave as `return 9999` looks so extremely hacky.
I.e., I understand `return <magic number>` is a stop-gap, but why can we not use `return Invalid` instead as a stop-gap? I do see the point about conflating legalization and cost-modelling, but I don't that is really important here as we are talking about plugging a whole, not about a decision decision that we want to keep. Thus, I don't think we would be conflating things if we return Invalid here, if that works, and I would also vote for that. The comments where `return 9999` is done has already the required explanations what is going on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103882