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

David Sherwood via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 26 02:34:05 PST 2020

david-arm added inline comments.

Comment at: llvm/include/llvm/Analysis/InstructionCost.h:224-236
+inline InstructionCost operator*(const InstructionCost &LHS,
+                                 const InstructionCost &RHS) {
+  InstructionCost LHS2(LHS);
+  LHS2 *= RHS;
+  return LHS2;
nhaehnle wrote:
> Dimensional analysis suggests that this particular overload of `operator*` should not exist. Instead, we should have `(unsigned, InstructionCost)` and `(InstructionCost, unsigned)` overloads. By dimensional analysis I mean that we should think of InstructionCost as having an implicit unit, such as ns or nJ or whatever. Multiplying two InstructionCosts would give us square seconds or square joules, which is nonsensical.
> Similarly, the return type of this particular overload of `operator/` should just be unsigned.
> Similar logic applies for the assignment operators.
Fundamentally the InstructionCost is really just a wrapper for an integer with an additional state attached. We're multiplying two integers, i.e. just the same as builtin operator*(int,int). The only difference between a traditional integer operator* and the InstructionCost variant is that we are propagating a state - see the propagateState() function.

We also tried to avoid complexities of modelling it too much like an FP value by talking about 'infinity' (which leads to the question as to what 0 * Infinity means). Instead, this class is more simple and pragmatic, which turns out to be sufficient for all uses of this class. If one of the values in an expression is Invalid, the resulting expression is Invalid as well.



More information about the llvm-commits mailing list