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

[Christopher Tetreault via Phabricator via llvm-commits]

ctetreau added a comment.

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.



================
Comment at: llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp:2155
   // inlining.
-  unsigned Cost = 0;
+  InstructionCost Cost = 0;
   for (auto *BB : L->getBlocks()) {
----------------
sdesmalen wrote:
> paulwalker-arm wrote:
> > More of an open question but is this initialisation required/preferred? Or to put another way, what is the expected meaning of InstructionCost()? Invalid? Zero?
> The default `InstructionCost()` initialises with a valid `0`. I would prefer to remove that constructor in favour of initializing it with an explicit value (when this is used for an accumulating cost value) or Invalid when used as 'uninitialized'. The only issue is that this requires some more changes, because it's used in a `std::pair<>` in e.g. LoopVectorize.cpp, where it needs a default constructor.
I don't like the default constructor either. I think a default constructed InstructionCost should be invalid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97466