[PATCH] D45150: Less conservative LoopSafetyInfo for headers

[Philip Reames via Phabricator via llvm-commits]

reames requested changes to this revision.
reames added a comment.
This revision now requires changes to proceed.

Specifically NOT okay to land as is.  The quadratic complexity concern is a real one and the window chosen here is very likely too large.

And if you're concern is purely LICM, this patch is no longer needed at all.  I introduced a surgical fix for LICM which solves this without the need for O(n^2) work.  Unfortunately, that approach only works for LICM (not, store promotion or other users of MustExecute).  Unfortunately, as you've found there don't appear to be any good cheap ways to ask ordering questions around instructions within a basic block.  If there were, we could just keep track of the first throwing instruction.

You could consider trying to wire in the caching needed for OrderedBasicBlock, but again, that's only needed if you're concern is different consumer than LICM



================
Comment at: llvm/lib/Analysis/MustExecute.cpp:263
+
+bool LoopSafetyInfo::isHeaderInstructionGuaranteedToExecute(
+    const Instruction *Instr, const Loop *CurLoop) const {
----------------
Purely code style: this duplicates similar code in ValueTracking.  Even if we were okay going this direction, we should merge not duplicate.


Repository:
  rL LLVM

https://reviews.llvm.org/D45150