[PATCH] D28756: Rewrite part of how loop ID is obtained.
Xin Tong via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 17 10:03:35 PST 2017
trentxintong added a comment.
In https://reviews.llvm.org/D28756#647388, @hfinkel wrote:
> In https://reviews.llvm.org/D28756#647380, @trentxintong wrote:
> > In https://reviews.llvm.org/D28756#647368, @sanjoy wrote:
> > > Code-wise this looks fine, but the "If a loop has multiple backedges and all of them have the same metadata we return that metadata. Otherwise we return nullptr." bit should be documented in the LangRef. I'll also ask you to please wait for someone familiar with how LLVM uses `!llvm.loop` to give a final go head before checking this in.
> > Thanks for the review. I will wait for @hfinkel to give the final approval.
> I think that this makes sense. I agree with Sanjoy that we should make sure that the LangRef reflects what we actually do here.
To be honest, I am a little bit confused now. Originally, I created this patch because according to getLoopID()'s comment, we should return the null if there are multiple latches and their loop IDs do not match (in case some are null, we return null). I am looking through how llvm.loop is used in the optimizer and came across this https://reviews.llvm.org/D26495 . we set the loop id to the newly created backedge in loopsimplify even some latches do not or have distinct loop ID's. These 2 things seem conflicting to me, i.e. on one side, we find and use the loop ID only if all the latches have them and they are identical, on the other side, we set loop ID (potentially use later) to the newly created backedge even though some latches do not have a loop id or have different loop IDs.
So which one is right ? Given a loop with multiple latches, and they have different loop IDs, can we use the loop ID and which one we use ?
More information about the llvm-commits