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

[Christopher Tetreault via Phabricator via llvm-commits]

ctetreau added a comment.

This is pretty much there, just a few nits and it will be good to go.



================
Comment at: llvm/include/llvm/Analysis/InstructionCost.h:34
+
+  typedef int CostType;
+
----------------
Since CostType is exposed through the public interface, it should be public.

NIT: prefer `using` to `typedef`


================
Comment at: llvm/include/llvm/Analysis/InstructionCost.h:40-43
+  void propagateState(const InstructionCost &RHS) {
+    if (State < RHS.State)
+      State = RHS.State;
+  }
----------------
NIT: I think it'd be clearer if we just checked that the state was invalid, especially since there are currently only two enumerants for it.


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

https://reviews.llvm.org/D91174