[PATCH] D91174: [Analysis] Introduce a new InstructionCost class

[David Sherwood via Phabricator via llvm-commits]

david-arm added a comment.

I think the main reason for having isValid (or something like that) was because it sometimes made control flow easier and there are places in the codebase today where all we want to do is assert that something is valid. I guess the alternative to calling isValid is something like:

  assert(Cost.getValue().hasValue() && "Cost is invalid!");
  return Cost;

or if we had a new getCostState() it would look like:

  assert(Cost.getCostState() == InstructionCost::Valid && "Cost is invalid!");
  return Cost;

which does work, but feels a bit less user-friendly. However, I do think that perhaps getting rid of isInvalid might make sense and introducing a getCostState() function. Perhaps I've missed something - any thoughts?


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

https://reviews.llvm.org/D91174