[PATCH] D123799: [RISCV] Mark FSIN and other math functions as Expand for scalable vectors.

Fraser Cormack via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 20 00:31:56 PDT 2022


frasercrmck added a comment.

In D123799#3461124 <https://reviews.llvm.org/D123799#3461124>, @craig.topper wrote:

> In D123799#3461104 <https://reviews.llvm.org/D123799#3461104>, @frasercrmck wrote:
>
>> I agree they should probably be invalid, conceptually, but in reality I don't like that we have users of cost models crash when encountering invalid costs, with the rationale that the IR shouldn't contain anything with an invalid cost. I don't see how those two models are compatible.
>
> Invalid cost is a recent concept. Did the project fail to update some code to comprehend the possibility of invalid costs?

To be honest, I don't have a good idea about the broader community view on invalid costs so I can't say for sure. I'm sure I've seen it argued in a review that invalid costs are a symptom of invalid IR which should have been legalized before reaching whichever part of code crashed. I can't find where I thought I saw that, though. I might have dreamed it.

`CodeMetrics` is one that definitely hard-crashes on invalid costs, and that's used by several optimizations that form the standard pipeline.

It's doubly hard for scalable-vector targets because many of the base TTIs assume fixed-length vectors and I've seen it argued in D119529 <https://reviews.llvm.org/D119529> (maybe where @liaolucy is coming from) that that's acceptable. We can't assume anything //other// than invalid costs for scalable vectors in base TTI functions. But then, in my opinion, forcing targets to fully cost everything before things stop crashing is asking too much.

On reflection, it's the combination of these two issues that's making life hard. We have to fully cost everything scalable but we also can't return invalid costs to get there without making assumptions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123799



More information about the llvm-commits mailing list