<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Apr 27, 2016, at 3:22 PM, David Blaikie via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Wed, Apr 27, 2016 at 12:46 PM, Thomas Jablin <span dir="ltr" class=""><<a href="mailto:tjablin@gmail.com" target="_blank" class="">tjablin@gmail.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class=""><div class=""><div class="">The short answer is, "Yes." <br class=""></div><div class=""><br class="">If a DbgInfoIntrinsic extended the live range of Value before LoopRotation, then LoopRotation may create a situation where the definition of the Value no dominates the DbgInfoIntrinsic. If so, the proposed patch will insert a PHINode.<br class=""><br class=""></div>If DbgInfoIntrinsics can extend the live range of a Value, then register allocation and other aspects of optimization and code generation will inevitably change. The alternative is pruning DbgInfoIntrinsics if they use Values that no longer dominate them and when they would extend the live-range of a Value.<br class=""></div></div></blockquote><div class=""><br class=""></div><div class="">I believe this (alternative) ^ would be necessary. I don't know if the right thing to do is prune the intrinsic, or just replace its value with undef, etc. I forget/don't know how we usually do it.</div></div></div></div></div></blockquote><div><br class=""></div><div>Let me know if I misunderstand something here, but AFAIK the register allocator goes out of its way to treat debug intrinsics as transparent.</div><div><br class=""></div><div>-- adrian</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class=""><div class=""><br class=""></div>If the DbgInfoIntrinsic is inside the live-range of a Value and does not extend it, no additional PHINodes will be need to be inserted.<br class=""></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br class=""><div class="gmail_quote">On Wed, Apr 27, 2016 at 1:27 PM, David Blaikie <span dir="ltr" class=""><<a href="mailto:dblaikie@gmail.com" target="_blank" class="">dblaikie@gmail.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="">Does this cause changes in non-debug IR in the presence of debug info intrinsics? (you mention that "<span style="font-family:monospace" class="">If there were uses of values produced by these instructions that were outside the loop, we have to insert PHI nodes to merge the two values." - that sounds like it could be a problem, as enabling debug info is not intended to change optimization/code generation)</span></div><div class="gmail_extra"><br class=""><div class="gmail_quote"><div class=""><div class="">On Tue, Apr 26, 2016 at 3:34 PM, Thomas Jablin via llvm-commits <span dir="ltr" class=""><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a>></span> wrote:<br class=""></div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class=""><div class="">tjablin created this revision.<br class="">
tjablin added reviewers: kbarton, hfinkel, cycheng.<br class="">
tjablin added a subscriber: llvm-commits.<br class="">
Herald added a subscriber: mzolotukhin.<br class="">
<br class="">
Loop rotation clones instruction from the old header into the preheader. If there were uses of values produced by these instructions that were outside the loop, we have to insert PHI nodes to merge the two values. If the values are used by DbgIntrinsics they will be used as a MetadataAsValue of a ValueAsMetadata of the original values, and iterating all of the uses of the original value will not update the DbgIntrinsics. The new code checks if the values are used by DbgIntrinsics and if so, updates them using essentially the same logic as the original code.<br class="">
<br class="">
The attached testcase demonstrates the issue. Without the fix, the DbgIntrinic outside the loop uses values computed inside the loop, even though these values do not dominate the DbgIntrinsic.<br class="">
<br class="">
<a href="http://reviews.llvm.org/D19564" rel="noreferrer" target="_blank" class="">http://reviews.llvm.org/D19564</a><br class="">
<br class="">
Files:<br class="">
lib/Transforms/Scalar/LoopRotation.cpp<br class="">
test/Transforms/LoopRotate/dbgvalue.ll<br class="">
<br class="">
Index: test/Transforms/LoopRotate/dbgvalue.ll<br class="">
===================================================================<br class="">
--- test/Transforms/LoopRotate/dbgvalue.ll<br class="">
+++ test/Transforms/LoopRotate/dbgvalue.ll<br class="">
@@ -7,6 +7,7 @@<br class="">
; CHECK-LABEL: define i32 @tak(<br class="">
; CHECK: entry<br class="">
; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 %x<br class="">
+; CHECK: tail call void @llvm.dbg.value(metadata i32 %call<br class="">
<br class="">
entry:<br class="">
br label %tailrecurse<br class="">
@@ -37,6 +38,44 @@<br class="">
ret i32 %<a href="http://z.tr/" rel="noreferrer" target="_blank" class="">z.tr</a>, !dbg !17<br class="">
}<br class="">
<br class="">
+define i32 @tak2(i32 %x, i32 %y, i32 %z) nounwind ssp !dbg !0 {<br class="">
+; CHECK-LABEL: define i32 @tak2(<br class="">
+; CHECK: entry<br class="">
+; CHECK: tail call void @llvm.dbg.value(metadata i32 %<a href="http://x.tr/" rel="noreferrer" target="_blank" class="">x.tr</a><br class="">
+; CHECK: tail call void @llvm.dbg.value(metadata i32 %<a href="http://x.tr/" rel="noreferrer" target="_blank" class="">x.tr</a><br class="">
+<br class="">
+entry:<br class="">
+ br label %tailrecurse<br class="">
+<br class="">
+tailrecurse: ; preds = %if.then, %entry<br class="">
+ %<a href="http://x.tr/" rel="noreferrer" target="_blank" class="">x.tr</a> = phi i32 [ %x, %entry ], [ %call, %if.then ]<br class="">
+ %<a href="http://y.tr/" rel="noreferrer" target="_blank" class="">y.tr</a> = phi i32 [ %y, %entry ], [ %call9, %if.then ]<br class="">
+ %<a href="http://z.tr/" rel="noreferrer" target="_blank" class="">z.tr</a> = phi i32 [ %z, %entry ], [ %call14, %if.then ]<br class="">
+ %cmp = icmp slt i32 %<a href="http://y.tr/" rel="noreferrer" target="_blank" class="">y.tr</a>, %<a href="http://x.tr/" rel="noreferrer" target="_blank" class="">x.tr</a>, !dbg !12<br class="">
+ br i1 %cmp, label %if.then, label %if.end, !dbg !12<br class="">
+<br class="">
+if.then: ; preds = %tailrecurse<br class="">
+ tail call void @llvm.dbg.value(metadata i32 %<a href="http://x.tr/" rel="noreferrer" target="_blank" class="">x.tr</a>, i64 0, metadata !6, metadata !DIExpression()), !dbg !7<br class="">
+ tail call void @llvm.dbg.value(metadata i32 %<a href="http://y.tr/" rel="noreferrer" target="_blank" class="">y.tr</a>, i64 0, metadata !8, metadata !DIExpression()), !dbg !9<br class="">
+ tail call void @llvm.dbg.value(metadata i32 %<a href="http://z.tr/" rel="noreferrer" target="_blank" class="">z.tr</a>, i64 0, metadata !10, metadata !DIExpression()), !dbg !11<br class="">
+ %sub = sub nsw i32 %<a href="http://x.tr/" rel="noreferrer" target="_blank" class="">x.tr</a>, 1, !dbg !14<br class="">
+ %call = tail call i32 @tak(i32 %sub, i32 %<a href="http://y.tr/" rel="noreferrer" target="_blank" class="">y.tr</a>, i32 %<a href="http://z.tr/" rel="noreferrer" target="_blank" class="">z.tr</a>), !dbg !14<br class="">
+ %sub6 = sub nsw i32 %<a href="http://y.tr/" rel="noreferrer" target="_blank" class="">y.tr</a>, 1, !dbg !14<br class="">
+ %call9 = tail call i32 @tak(i32 %sub6, i32 %<a href="http://z.tr/" rel="noreferrer" target="_blank" class="">z.tr</a>, i32 %<a href="http://x.tr/" rel="noreferrer" target="_blank" class="">x.tr</a>), !dbg !14<br class="">
+ %sub11 = sub nsw i32 %<a href="http://z.tr/" rel="noreferrer" target="_blank" class="">z.tr</a>, 1, !dbg !14<br class="">
+ %call14 = tail call i32 @tak(i32 %sub11, i32 %<a href="http://x.tr/" rel="noreferrer" target="_blank" class="">x.tr</a>, i32 %<a href="http://y.tr/" rel="noreferrer" target="_blank" class="">y.tr</a>), !dbg !14<br class="">
+ br label %tailrecurse<br class="">
+<br class="">
+if.end: ; preds = %tailrecurse<br class="">
+ tail call void @llvm.dbg.value(metadata i32 %<a href="http://x.tr/" rel="noreferrer" target="_blank" class="">x.tr</a>, i64 0, metadata !6, metadata !DIExpression()), !dbg !7<br class="">
+ tail call void @llvm.dbg.value(metadata i32 %<a href="http://y.tr/" rel="noreferrer" target="_blank" class="">y.tr</a>, i64 0, metadata !8, metadata !DIExpression()), !dbg !9<br class="">
+ tail call void @llvm.dbg.value(metadata i32 %<a href="http://z.tr/" rel="noreferrer" target="_blank" class="">z.tr</a>, i64 0, metadata !10, metadata !DIExpression()), !dbg !11<br class="">
+ br label %return, !dbg !16<br class="">
+<br class="">
+return: ; preds = %if.end<br class="">
+ ret i32 %<a href="http://z.tr/" rel="noreferrer" target="_blank" class="">z.tr</a>, !dbg !17<br class="">
+}<br class="">
+<br class="">
@channelColumns = external global i64<br class="">
@horzPlane = external global i8*, align 8<br class="">
<br class="">
Index: lib/Transforms/Scalar/LoopRotation.cpp<br class="">
===================================================================<br class="">
--- lib/Transforms/Scalar/LoopRotation.cpp<br class="">
+++ lib/Transforms/Scalar/LoopRotation.cpp<br class="">
@@ -108,6 +108,33 @@<br class="">
<br class="">
// Anything else can be handled by SSAUpdater.<br class="">
SSA.RewriteUse(U);<br class="">
+<br class="">
+ // Replace MetadataAsValue(ValueAsMetadata(OrigHeaderVal)) uses in debug<br class="">
+ // intrinsics<br class="">
+ LLVMContext &C = OrigHeader->getContext();<br class="">
+ if (ValueAsMetadata *VAM = ValueAsMetadata::getIfExists(OrigHeaderVal)) {<br class="">
+ if (MetadataAsValue *MAV = MetadataAsValue::getIfExists(C, VAM)) {<br class="">
+ for (auto UI = MAV->use_begin(), E = MAV->use_end(); UI != E; ) {<br class="">
+ // Grab the use before incrementing the iterator.<br class="">
+ Use &U = *UI++;<br class="">
+ DbgInfoIntrinsic *UserInst = cast<DbgInfoIntrinsic>(U.getUser());<br class="">
+ BasicBlock *UserBB = UserInst->getParent();<br class="">
+<br class="">
+ // The original users in the OrigHeader are already using the<br class="">
+ // original definitions.<br class="">
+ if (UserBB == OrigHeader)<br class="">
+ continue;<br class="">
+<br class="">
+ // Users in the OrigPreHeader need to use the value to which the<br class="">
+ // original definitions are mapped and anything else can be handled<br class="">
+ // by the SSAUpdater<br class="">
+ Value *NewVal = UserBB == OrigPreheader ?<br class="">
+ OrigPreHeaderVal :<br class="">
+ SSA.GetValueInMiddleOfBlock(UserBB);<br class="">
+ U = MetadataAsValue::get(C, ValueAsMetadata::get(NewVal));<br class="">
+ }<br class="">
+ }<br class="">
+ }<br class="">
}<br class="">
}<br class="">
}<br class="">
<br class="">
<br class="">
<br class=""></div></div>_______________________________________________<br class="">
llvm-commits mailing list<br class="">
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a><br class="">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="">
<br class=""></blockquote></div><br class=""></div>
</blockquote></div><br class=""></div>
</div></div></blockquote></div><br class=""></div></div>
_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits<br class=""></div></blockquote></div><br class=""></body></html>