[PATCH] D95353: NFC: Migrate SpeculateAroundPHIs to work on InstructionCost

[Christopher Tetreault via Phabricator via llvm-commits]

ctetreau added a comment.

Aside from the one nit, this looks good to me.



================
Comment at: llvm/lib/Transforms/Scalar/SpeculateAroundPHIs.cpp:329
                                             "the sum must be as well.");
-
+  assert(TotalMatCost.isValid() && "Constants must be  materializable");
   LLVM_DEBUG(dbgs() << "    Cost savings " << (TotalMatCost - TotalFoldedCost)
----------------
I assume this assert is to keep this patch NFC? If so, please reword the message to imply that it should be possible for this to happen, but it's not implemented yet or add a FIXME comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95353