[PATCH] D97382: NFC: Migrate PartialInlining to work on InstructionCost

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen marked 7 inline comments as done.
sdesmalen added inline comments.


================
Comment at: llvm/include/llvm/Support/InstructionCost.h:205-209
+  InstructionCost map(const std::function<CostType(const CostType &)> &F) {
+    if (isValid())
+      return InstructionCost(F(*getValue()));
+    return getInvalid();
+  }
----------------
ctetreau wrote:
> ctetreau wrote:
> > Having the function type be a template parameter (as it is defined in Optional.h) is preferred because lambda types can be directly used without the indirection of assigning them to a std::function. It also avoids having to `#include <functional>`
> also, I believe map can be const.
> Having the function type be a template parameter (as it is defined in Optional.h) is preferred because lambda types can be directly used without the indirection of assigning them to a std::function. It also avoids having to #include <functional>
Okay, I wasn't sure if it was better to limit the signature of Function by specifying it would take/return a CostType by using std::function. Any issues resulting from the lambda return the wrong value type probably show up when calling the constructor of InstructionCost.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97382