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

[Christopher Tetreault via Phabricator via llvm-commits]

ctetreau added inline comments.


================
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)
----------------
sdesmalen wrote:
> ctetreau wrote:
> > 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.
> Not entirely; if a Constant cannot be folded when speculating, it must be possible to materialize it fully, and so we must be able to cost it. Otherwise there is something wrong with the existing IR. I don't think we should ever encounter this case.
If a user writes bad IR, the system shouldn't crash. If this is truly impossible except in cases of bad IR, then we should raise a fatal error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95353