[PATCH] D53744: [SimpleLoopUnswitch] Unswitch by experimental.guard intrinsics

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:2253-2255
+  // TODO: We don't know for sure that this guard will then be unswitched. We
+  // can make this statistics more accurate.
+  ++NumGuards;
----------------
mkazantsev wrote:
> chandlerc wrote:
> > What happens if the guard isn't unswitched? Will anything clean up the branch?
> Nope. As far as I'm aware, `unswitchNontrivialInvariants` can only return `false` if one of successors starts with `CleanupPadInst`, which is not the case we have after this split. I believe that in practice unswitching should be always successful after this transform. I placed a `TODO` to follow-up on this in the future (I think we need to assert that fact).
The only way how unswitch may fail is marked as FIXME:

  // We cannot unswitch if exit blocks contain a cleanuppad instruction as we
  // don't know how to split those exit blocks.
  // FIXME: We should teach SplitBlock to handle this and remove this
  // restriction.
  for (auto *ExitBB : ExitBlocks)
    if (isa<CleanupPadInst>(ExitBB->getFirstNonPHI()))
      return false;

I think it's more or less OK to not heal the guard if unswitching failed, provided that there is a plan to either make it unfailable or migrate to new guards representation in D51207.


https://reviews.llvm.org/D53744