[PATCH] D79162: [Analysis] TTI: Add CastContextHint for getCastInstrCost

Pierre van Houtryve via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 5 06:25:54 PDT 2020


Pierre-vh added a comment.

In D79162#2017965 <https://reviews.llvm.org/D79162#2017965>, @dmgreen wrote:

> In D79162#2014497 <https://reviews.llvm.org/D79162#2014497>, @Pierre-vh wrote:
>
> > Yes, I originally tried an enum with one entry per "kind" of load/store, as you said. It was working fine, but I felt that it was a bit confusing, as each entry had a different meaning based on the opcode. For instance, I had the `MaskedVector` entry, which, for most casts, meant that the operand was a masked load, but for truncs meant that the single user of the cast is a masked store. ,What if another target needs to deal with truncs of masked loads or something like that ? They wouldn't be able to use that API, they'd have to hack it. (I don't know if this will ever come up, I'm just trying to think about all of the use cases for this enum)
> >  With the current format of the enum, it's a clearer IMHO, there is no ambiguity, and any target can add their specific case in there and deal with it in the backend.
> >
> > Of course, both versions work equally well, I'm fine with both.
>
>
> I feel like a truncating load is not a very common thing. We should try and get the common case working first. Although I see your point about the other types of casts, it might be enough at the moment to only look at sext, zext and trunc. Trunc is a little different because we are looking at the operands, might not be the prettiest, but otherwise I think should be OK.
>
> I think that we in MVE need a way to distinguish all the types of loads/stores that the vectorizer produces, even if the context instruction is incorrect and cannot be trusted. But it may be better not to think of this from an individual backend perspective exactly. We are trying to pass the information that the midend knows through to the costmodel. In that way it makes sense to me to add a parameter that has vales {None, Normal, Masked, Interleaved and Gather}, maybe reversed too as the vectorizer can produce it. Get them to be passed through to the correct places, or calculated from the context if nothing else is known. That sounds like it should be able to be done cleanly and simply enough.
>
> I believe that extending load and truncating stores are at least the most common ones we need to worry about, (if not the only one's). Can give that a go, see how it looks?


If I understand correctly, I should:

- Use {None, Normal, Masked, Interleaved and Gather} instead of the current enum values. (No Scatter? e.g. for trunc to scatter store? Do I need the "reversed" one as well?)
- Only set `CastContextHint` for zext, sext and trunc.

Is that correct? If yes, I can give it a go and we'll see how it looks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79162





More information about the llvm-commits mailing list