[PATCH] D92238: [SCEVExpander] Migrate costAndCollectOperands to use InstructionCost.

[Christopher Tetreault via Phabricator via llvm-commits]

ctetreau added a comment.

In D92238#2521095 <https://reviews.llvm.org/D92238#2521095>, @lebedev.ri wrote:

> Won't we now not stop as soon as the budget is invalidated?
> Perhaps if `InstructionCost::operator<` can't be fixed, it needs to be ripped out.
> I'm sorry, i don't have useful feedback other than "the new abstraction is bad".

I agree that it's strictly worse in that the early return is not triggered if anything returns an invalid cost. I disagree that `InstructionCost::operator<` needs to be fixed or that "the new abstraction is bad".

The new abstraction has a well-defined semantics that is useful in the common case of deciding which of two costs is preferable. Throughout the codebase, we decide if `thingA` should be done over `thingB` by asking if the cost of `thingA` is less than the cost of `thingB`. If one of those things has an invalid cost, then it makes sense to consider the thing with a valid cost as less than it. If they are both invalid, then it doesn't really matter which one we pick.

I'm curious: what's the objection to having the budget be a negative number, and adding costs instead of subtracting? We could also accumulate costs in a separate variable and compare `Cost < Budget`, which will also do the right thing in the face of an invalid cost infecting the running total. In this case, `Budget` doesn't even need to be an `InstructionCost`.


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

https://reviews.llvm.org/D92238