[PATCH] D95803: Ensure that InstructionCost actually implements a total ordering

[Christopher Tetreault via Phabricator via llvm-commits]

ctetreau added inline comments.


================
Comment at: llvm/include/llvm/Support/InstructionCost.h:155
+  bool operator<(const InstructionCost &RHS) const {
+    return State < RHS.State || Value < RHS.Value;
+  }
----------------
nikic wrote:
> Shouldn't this be?
> ```
> if (State != RHS.State)
>   return State < RHS.State;
> return Value < RHS.Value;
> ```
> If State is Invalid and RHS.State is Valid, then the Value comparison shouldn't be coming into it. (I assume the Value is not even initialized in that case, which is why this patch caused major compiler non-determinism.)
Ugh, yeah, I believe you are correct. With this definition, (invalid, 10) < (valid, 5) ==> true

I believe the value is always initialized, so this should be deterministic. It just gives the wrong answer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95803