[PATCH] D19564: Update Debug Intrinsics in RewriteUsesOfClonedInstructions in LoopRotation
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 27 15:22:28 PDT 2016
On Wed, Apr 27, 2016 at 12:46 PM, Thomas Jablin <tjablin at gmail.com> wrote:
> The short answer is, "Yes."
>
> 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.
>
> 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.
>
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.
>
> 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.
>
> On Wed, Apr 27, 2016 at 1:27 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>> Does this cause changes in non-debug IR in the presence of debug info
>> intrinsics? (you mention that "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)
>>
>> On Tue, Apr 26, 2016 at 3:34 PM, Thomas Jablin via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> tjablin created this revision.
>>> tjablin added reviewers: kbarton, hfinkel, cycheng.
>>> tjablin added a subscriber: llvm-commits.
>>> Herald added a subscriber: mzolotukhin.
>>>
>>> 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.
>>>
>>> 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.
>>>
>>> http://reviews.llvm.org/D19564
>>>
>>> Files:
>>> lib/Transforms/Scalar/LoopRotation.cpp
>>> test/Transforms/LoopRotate/dbgvalue.ll
>>>
>>> Index: test/Transforms/LoopRotate/dbgvalue.ll
>>> ===================================================================
>>> --- test/Transforms/LoopRotate/dbgvalue.ll
>>> +++ test/Transforms/LoopRotate/dbgvalue.ll
>>> @@ -7,6 +7,7 @@
>>> ; CHECK-LABEL: define i32 @tak(
>>> ; CHECK: entry
>>> ; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 %x
>>> +; CHECK: tail call void @llvm.dbg.value(metadata i32 %call
>>>
>>> entry:
>>> br label %tailrecurse
>>> @@ -37,6 +38,44 @@
>>> ret i32 %z.tr, !dbg !17
>>> }
>>>
>>> +define i32 @tak2(i32 %x, i32 %y, i32 %z) nounwind ssp !dbg !0 {
>>> +; CHECK-LABEL: define i32 @tak2(
>>> +; CHECK: entry
>>> +; CHECK: tail call void @llvm.dbg.value(metadata i32 %x.tr
>>> +; CHECK: tail call void @llvm.dbg.value(metadata i32 %x.tr
>>> +
>>> +entry:
>>> + br label %tailrecurse
>>> +
>>> +tailrecurse: ; preds = %if.then,
>>> %entry
>>> + %x.tr = phi i32 [ %x, %entry ], [ %call, %if.then ]
>>> + %y.tr = phi i32 [ %y, %entry ], [ %call9, %if.then ]
>>> + %z.tr = phi i32 [ %z, %entry ], [ %call14, %if.then ]
>>> + %cmp = icmp slt i32 %y.tr, %x.tr, !dbg !12
>>> + br i1 %cmp, label %if.then, label %if.end, !dbg !12
>>> +
>>> +if.then: ; preds = %tailrecurse
>>> + tail call void @llvm.dbg.value(metadata i32 %x.tr, i64 0, metadata
>>> !6, metadata !DIExpression()), !dbg !7
>>> + tail call void @llvm.dbg.value(metadata i32 %y.tr, i64 0, metadata
>>> !8, metadata !DIExpression()), !dbg !9
>>> + tail call void @llvm.dbg.value(metadata i32 %z.tr, i64 0, metadata
>>> !10, metadata !DIExpression()), !dbg !11
>>> + %sub = sub nsw i32 %x.tr, 1, !dbg !14
>>> + %call = tail call i32 @tak(i32 %sub, i32 %y.tr, i32 %z.tr), !dbg !14
>>> + %sub6 = sub nsw i32 %y.tr, 1, !dbg !14
>>> + %call9 = tail call i32 @tak(i32 %sub6, i32 %z.tr, i32 %x.tr), !dbg
>>> !14
>>> + %sub11 = sub nsw i32 %z.tr, 1, !dbg !14
>>> + %call14 = tail call i32 @tak(i32 %sub11, i32 %x.tr, i32 %y.tr), !dbg
>>> !14
>>> + br label %tailrecurse
>>> +
>>> +if.end: ; preds = %tailrecurse
>>> + tail call void @llvm.dbg.value(metadata i32 %x.tr, i64 0, metadata
>>> !6, metadata !DIExpression()), !dbg !7
>>> + tail call void @llvm.dbg.value(metadata i32 %y.tr, i64 0, metadata
>>> !8, metadata !DIExpression()), !dbg !9
>>> + tail call void @llvm.dbg.value(metadata i32 %z.tr, i64 0, metadata
>>> !10, metadata !DIExpression()), !dbg !11
>>> + br label %return, !dbg !16
>>> +
>>> +return: ; preds = %if.end
>>> + ret i32 %z.tr, !dbg !17
>>> +}
>>> +
>>> @channelColumns = external global i64
>>> @horzPlane = external global i8*, align 8
>>>
>>> Index: lib/Transforms/Scalar/LoopRotation.cpp
>>> ===================================================================
>>> --- lib/Transforms/Scalar/LoopRotation.cpp
>>> +++ lib/Transforms/Scalar/LoopRotation.cpp
>>> @@ -108,6 +108,33 @@
>>>
>>> // Anything else can be handled by SSAUpdater.
>>> SSA.RewriteUse(U);
>>> +
>>> + // Replace MetadataAsValue(ValueAsMetadata(OrigHeaderVal)) uses
>>> in debug
>>> + // intrinsics
>>> + LLVMContext &C = OrigHeader->getContext();
>>> + if (ValueAsMetadata *VAM =
>>> ValueAsMetadata::getIfExists(OrigHeaderVal)) {
>>> + if (MetadataAsValue *MAV = MetadataAsValue::getIfExists(C,
>>> VAM)) {
>>> + for (auto UI = MAV->use_begin(), E = MAV->use_end(); UI != E;
>>> ) {
>>> + // Grab the use before incrementing the iterator.
>>> + Use &U = *UI++;
>>> + DbgInfoIntrinsic *UserInst =
>>> cast<DbgInfoIntrinsic>(U.getUser());
>>> + BasicBlock *UserBB = UserInst->getParent();
>>> +
>>> + // The original users in the OrigHeader are already using
>>> the
>>> + // original definitions.
>>> + if (UserBB == OrigHeader)
>>> + continue;
>>> +
>>> + // Users in the OrigPreHeader need to use the value to
>>> which the
>>> + // original definitions are mapped and anything else can be
>>> handled
>>> + // by the SSAUpdater
>>> + Value *NewVal = UserBB == OrigPreheader ?
>>> + OrigPreHeaderVal :
>>> + SSA.GetValueInMiddleOfBlock(UserBB);
>>> + U = MetadataAsValue::get(C, ValueAsMetadata::get(NewVal));
>>> + }
>>> + }
>>> + }
>>> }
>>> }
>>> }
>>>
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160427/0997d063/attachment.html>
More information about the llvm-commits
mailing list