[PATCH] D46422: [LCSSA] Iteratively remove unused PHI nodes in formLCSSAForInstructions

Michael Zolotukhin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 4 11:58:58 PDT 2018


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

Hi,

First of all, please clean-up the testcase. The bugpoint-output tests are impossible to understand and maintain (it is impossible to say if such a test checks anything at all after some time). I'm pretty sure you can expose the same issue with much a smaller test.

Second, why don't just replace

  assert (PN->use_empty() && "Trying to remove a phi with uses.");

with

  if (PN->use_empty())
    continue;

?
When this code was written the assumption was that it's impossible to have a Phi-node with uses in that list. If your test case proves this to be wrong, then replace the assertion with a check.

Thanks,
Michael



================
Comment at: lib/Transforms/Utils/LCSSA.cpp:233-235
+  while (RemovePHIs) {
+    RemovePHIs = false;
+    SmallSetVector<PHINode *, 16> NotRemoved;
----------------
Could we ever iterate this loop more than once? How could we have phi-nodes to be deleted on the second iteration (~why were not they removed on the first one)?


Repository:
  rL LLVM

https://reviews.llvm.org/D46422





More information about the llvm-commits mailing list