[PATCH] D97466: NFC: Change getUserCost to return InstructionCost

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

In D97466#2590977 <https://reviews.llvm.org/D97466#2590977>, @ctetreau wrote:

> Does changing the -1's to invalid's constitute a functional change? Are they always checked for? If they are just blindly added to other costs, then they would result in valid costs becoming invalid costs.

In TargetTransformInfo::getUserCost (in TargetTransformInfo.cpp) it has the following check:

  InstructionCost Cost = TTIImpl->getUserCost(U, Operands, CostKind);
  assert((CostKind == TTI::TCK_RecipThroughput || Cost >= 0) &&
         "TTI should not produce negative costs!");

By returning Invalid the assert no longer fails but instead Invalid is propagated, which you are right is a functional change.
I'll revert to returning -1 instead of Invalid, and will leave that to a separate (non-NFC) patch.



================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfoImpl.h:1029
+        return CostKind == TTI::TCK_RecipThroughput
+                   ? InstructionCost::getInvalid()
+                   : 1;
----------------
dmgreen wrote:
> This seems strange. Does it need to return an invalid cost? Or can it just return a cost of 1 for all cost kinds?
For these patches, I haven't really tried to understand the reason behind why things have been cost-modelled the way they have, and have instead focussed on the interface-change alone. I agree returning -1 seems odd, I don't know why this cannot be costed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97466