[PATCH] D78937: [CostModel] Use isExtLoad in BasicTTI

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

In D78937#2100582 <https://reviews.llvm.org/D78937#2100582>, @samparker wrote:

> But now we're checking that the queried type matches the type of context instruction too, so I'm not really what other conclusion to draw other than that the user asking: what is the cost of //this// instruction? Based on the currently limited API, what scenario do you think this would not generally be true?


But you removed the code that would check if a the extend is free if they do not match up. So the vectorizer calling getCastCost with vector types but the original scalar instruction would give different results? I would imagine this might be where a lot of the X86 changes are coming from.

This function has always been in the past - "give me the cost of a hypothetical instruction, using I as context". It has never been "give me the cost of I".  I'm not really saying we shouldn't change that, I really just want fix some of the regressions we have already seen and improve how we can handle masked loads for tail predication. But I do think that the first makes for a better design if we can possibly stick to it. We might well want to change it to "give me the cost of this hypothetical instruction or this this given instruction I", but that means we would need more joined up thinking for the places calling this (vectorizer, slp, etc) to not pass I if the instructions do not match up. And that on it's own loses an amount of useful information.


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

https://reviews.llvm.org/D78937