[PATCH] D24805: [GVNSink] Initial GVNSink prototype

[James Molloy via Phabricator via llvm-commits]

jmolloy marked an inline comment as done.
jmolloy added a comment.

Thanks Danny,

I've committed the patch after changing from std::stable_sort to std::sort (I didn't need it, I was being paranoid debugging a heisenbug). This was good timing as today is my last day at ARM and in a couple of hours I will lose access to my development machine! :)

Thanks for your last reply; I now, finally, have got it! I spent yesterday prototyping and have something that conceptually works. It'll need a lot more work though. Watch this space ;)

Cheers,

James



================
Comment at: lib/Transforms/Scalar/GVNSink.cpp:75
+
+/// Iterates through instructions in a set of blocks in reverse order from the
+/// first non-terminator. For example (assume all blocks have size n):
----------------
dberlin wrote:
> I'm not sure how easy/possible it is to do, but isn't this just a zip_iterator of a bunch of reverse iterators?
> 
Similar; it'd be really cool to have something like zip_iterator in STLExtras. Although this does have some domain-specific utility functions, and doens't have undefined behaviour when the input iterators aren't of equal length :)


================
Comment at: lib/Transforms/Scalar/GVNSink.cpp:198
+      Blocks.push_back(PN->getIncomingBlock(I));
+    std::stable_sort(Blocks.begin(), Blocks.end());
+
----------------
dberlin wrote:
> I'm curious why you think you need stable sort here as opposed to regular
> 
I don't. Changed.


Repository:
  rL LLVM

https://reviews.llvm.org/D24805