[PATCH] D21776: MSSA Walker Pre-refactor

Daniel Berlin via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 27 19:08:13 PDT 2016


If we think dominates is necessary, go for it.

On Mon, Jun 27, 2016 at 6:19 PM, George Burgess IV <
george.burgess.iv at gmail.com> wrote:

> george.burgess.iv marked an inline comment as done.
>
> ================
> Comment at: lib/Transforms/Utils/MemorySSA.cpp:551
> @@ -550,3 +605,1 @@
> -}
> -
>  /// \brief Returns true if \p Replacer dominates \p Replacee .
> ----------------
> dberlin wrote:
> > I see you removed this - this is a utility being used as part of
> updating for another pass i haven't submitted yet.
> >
> >
> Sounds good; will unremove. Thanks for the heads up.
>
> ================
> Comment at: lib/Transforms/Utils/MemorySSA.cpp:856
> @@ -805,1 +855,3 @@
> +  // A node dominates itself, and liveOnEntry dominates everything.
> +  if (Dominatee == Dominator || isLiveOnEntryDef(Dominator))
>      return true;
> ----------------
> dberlin wrote:
> > This makes no sense unless the block we are talking about is the entry
> block.
> > Otherwise, how are they in the same block?
> >
> > (if it is the entry block, the loop below will give the correct answer,
> no?)
> This is just a refactor of the old (and the new changes `locallyDominates`
> to `dominates`, since the only user of `locallyDominates` that I could find
> was the current walker, so we'd probably readd
> `isLiveOnEntryDef(Dominator)` anyway, since it could save domtree stuff.).
>
> If we don't want to kill `locallyDominates`, I'm happy to remove this,
> because yes, the below loop should catch this case.
>
>
> http://reviews.llvm.org/D21776
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160627/bb3cf6e2/attachment.html>


More information about the llvm-commits mailing list