[PATCH] D72382: [ArgPromotion] Extend search for SafeToUnconditionallyLoad indices to all post-dominators of the entry block.

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert requested changes to this revision.
jdoerfert added a reviewer: jdoerfert.
jdoerfert added a comment.

Using post-dominator information is not safe. Take this code:

  static int foo(int c, int *a) {
    if (c) {
      while (1) {}
    } else {
      my_personal_exit();    // < no return
    }
    return *a; // < post dominator is dead
  }

What you can use is the must-be-executed context as it was introduced for this reason, e.g.,:

  MustBeExecutedContextExplorer Explorer(...);
  auto EIt = Explorer.begin(&EntryBB.front()), EEnd = Explorer.end(&EntryBB.front());
  do {
    Instruction *ExecutedI = EIt.getCurrentInst();
    // extract properties from ExecutedI.
  } while (++EIt != EEnd);

FWIW, I'm strongly in favor of removing ArgumentPromotion as a pass, the first step towards this goal is D68852 <https://reviews.llvm.org/D68852>, I just haven't had the time to rebase and merge it.
I want this for various reasons, including the bugs in ArgumentPromotion, which are fixable of course. (See the bugzilla PR42852, PR887, PR42683). Generally speaking,
a lot of things happening in this pass are, or should be, part of other analyses/transformations.

If you would be interested to make the pointer privatization in the Attributor stronger, I'd be happy to help with that.


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

https://reviews.llvm.org/D72382