[PATCH] D80399: [Local] Prevent `invertCondition` from creating a redundant instruction

Jay Foad via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 22 03:36:34 PDT 2020


foad added a comment.

Looks great to me!



================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:3067-3068
-    if (isa<PHINode>(Inst)) {
-      // FIXME: This fails if the inversion is to be used in a
-      // subsequent PHINode in the same basic block.
-      Inverted->insertBefore(&*Parent->getFirstInsertionPt());
----------------
sameerds wrote:
> I don't think the current change fixes this issue. As far as I can see, it can't even be fixed inside this function since the intended use is not visible here. It might help to move the comment to the beginning of the function so that it is more obvious to users.
I think the FIXME is completely misguided. PHIs are defined to happen simultaneously so there is no "subsequent PHINode".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80399





More information about the llvm-commits mailing list