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

[Pierre van Houtryve via Phabricator via llvm-commits]

Pierre-vh added a comment.

In D79162#2034911 <https://reviews.llvm.org/D79162#2034911>, @efriedma wrote:

> The general idea of passing down information about the instruction makes sense.
>
> The one thing I would say here is that we probably shouldn't pass down the "Instruction" from the vectorizer at all: in general, the cost computation doesn't know what sort of transforms the vectorizer is going to do, so any information computed from its operands/uses will be misleading in general.


I agree, I'll remove the instruction from the calls to `getCastInstrCost` in the vectorizer.
It was mostly useless with those changes anyway. I removed it, and tests are still passing just fine.

In D79162#2033897 <https://reviews.llvm.org/D79162#2033897>, @SjoerdMeijer wrote:

> Had a first quick look, and here's  a drive-by comment from my side: I can't say it was love at first sight for me with this patch. It indeed feels like a very narrow approach, and am not entirely sure if it is similar to getArithmeticInstrCost. I see the challenge here though, and haven't given it any time to think about an alternative.  Not sure, would it be worth to write to the dev list to get some more exposure/views/opinion on this?


I can't think of many alternatives though. We also thought about implementing `getMaskedMemoryOpCost` in the ARM backend, but it doesn't solve the issue nicely:

- The cost would be in the wrong place: it's not the load that's expensive, it's the extend/trunc. If you remove the cast, the problem disappears.
- We'd also need to change `getMaskedMemoryOpCost`. At the very least, it'll need to know the type after/before extend/trunc, which will need to be calculated in the vectorizer to account for the minimal bitwidths (e.g. sometimes, instructions such as zext i32 can become zext i16 after vectorization as the vectorizer know that the value doesn't need more than 16 bits)

I agree that sending a message on the mailing list (with a link to this patch) would be a good idea so we can get more feedback.


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

https://reviews.llvm.org/D79162