[PATCH] D18586: Allow DeadStoreElimination to track combinations of partial later wrties

[Erik Eckstein via llvm-commits]

eeckstein accepted this revision.
eeckstein added a comment.
This revision is now accepted and ready to land.

LGTM (but please wait for other reviewers)


================
Comment at: lib/Transforms/Scalar/DeadStoreElimination.cpp:457
@@ +456,3 @@
+    auto ILI = IM.find(LaterIntStart);
+    if (ILI != IM.end() && ILI.start() < LaterIntEnd) {
+      // This existing interval ends in the middle of
----------------
hfinkel wrote:
> eeckstein wrote:
> > Shouldn't this be
> >  if (ILI != IM.end() && ILI.stop() < LaterIntEnd) {
> > 
> > If yes, then what if the existing interval completely covers the Later-interval? I.e. if ILI.stop() >= LaterIntEnd
> We want to know if we have an interval that overlaps with the current one. IM.find(LaterIntStart) will return the first interval that ends at or after the current one's start. Next we need to check if the interval starts before the current one's end (to see whether it overlaps the current one, or is purely after it). Checking whether it stops before the current one ends is insufficient because it will miss the case where the interval crosses the current one's end point. I'll improve the comments.
Oh, I see. I misunderstood the find() function.


http://reviews.llvm.org/D18586