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

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

I was trying yesterday to rebase D79162 <https://reviews.llvm.org/D79162> on top of some fp16 work I have been doing lately, but that is not ready yet. I do think it makes sense to try and get those problems fixed first, and don't believe the changes here are really a good thing. It is dangerous with how we have things set up at the moment for anything to look at the _type_ of the context instruction.  It is wrong in too many places. Ideally you would only even look at the opcode of surrounding instructions, and even those are sometimes dubious.

If we do really want to do this, we would probably need to remove the context from any place where it wasn't the actual instruction being costed. That would have to be done without causing regressions anywhere else too. And so might need two code paths - the existing one that went though isLoadExtLegal and the call to isExtLoad.

As I've said elsewhere my idea in the long run is to somehow create a better framework for costing multiple instruction at the same time. Which would probably be related to for how vplan cost modelling turns out. This will likely not happen any time soon though.


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

https://reviews.llvm.org/D78937