[llvm-commits] patch: moving unreachable elimination to codegenprep

[Dan Gohman]

On 2 February 2010 21:21, Nick Lewycky <nicholas at mxc.ca> wrote:
> 
> Jakob, I'll make the tweaks to InlineCost that you proposed and post an
> updated patch with the results of a nightly test run. Realize that I'm
> expecting no visible performance difference at all since no code in the
> nightly test uses __builtin_unreachable().

There is code which uses things like "noreturn" and other
things which can lead to unreachable getting generated.
Offhand you may be right that these wouldn't be affected
though. This happens to be a very testable hypothesis :).

Would you mind compiling some tests with and without the
change, and diffing the .s files? I'd be interested in
learning what kinds of diffs, if any, appear.

I'm concerned that artificially keeping the condition
instructions live will hamper the optimizer. Your benchmark
results are good, however it's not clear if that's just a
consequence of an absence of  __builtin_unreachable or if
that actually says something about the impact of artificial
life on the optimizer.

One comment on the patch itself:

--- lib/Analysis/ScalarEvolution.cpp    (revision 95201)
+++ lib/Analysis/ScalarEvolution.cpp    (working copy)

+  // Ignore faux exits.
+  for (unsigned i = 0; i != ExitingBlocks.size(); ++i) {
+    if (isa<UnreachableInst>(ExitingBlocks[i]->front())) {
+      ExitingBlocks.erase(ExitingBlocks.begin() + i);
+      --i;
+    }
+  }

ExitingBlocks is an unsorted array. Instead of deleting blocks from
the middle of it, please swap the element to the end and pop_back().

Also, would it make sense to pull this code into the Loop(Base) class
itself? There is other code that looks at loop exists and probably
would need this.

Dan