[PATCH] D92576: Remove insertDebugValuesForPHIs() from LCSSA to prevent assignments from being reordered
Nabeel via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 3 07:18:00 PST 2020
n-omer created this revision.
n-omer added reviewers: jmorse, TWeaver, probinson, aprantl, chrisjackson, dblaikie.
n-omer added a project: debug-info.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.
n-omer requested review of this revision.
The LCSSA pass makes use of a function `insertDebugValuesForPHIs()` to propogate `dbg.value()` intrinsics to newly inserted PHI instructions.
Faulty behaviour occurs when the associated parent PHI of a newly inserted PHI is not the most recent assignment to a source variable as can be seen below:
; input.ll:
%S2 = ...
br label %loop.interior
loop.interior:
br i1 %S2, label %if.true, label %if.false
if.true:
%X1 = ...
@llvm.dbg.value(var = %X1)
br label %post.if
if.false:
%X2 = ...
@llvm.dbg.value(var = %X2)
br label %post.if
post.if:
%X3 = phi(X1, X2)
@llvm.dbg.value(var = %X3)
%Y1 = ...
@llvm.dbg.value(var = %Y1)
br i1 %S2, label %loop.exit, label %loop.interior
loop.exit:
... = X3 + 4
; output.ll:
%S2 = ...
br label %loop.interior
loop.interior:
br i1 %S2, label %if.true, label %if.false
if.true:
%X1 = ...
@llvm.dbg.value(var = %X1)
br label %post.if
if.false:
%X2 = ...
@llvm.dbg.value(var = %X2)
br label %post.if
post.if:
%X3 = phi(X1, X2)
@llvm.dbg.value(var = %X3)
%Y1 = ...
@llvm.dbg.value(var = %Y1)
br i1 %S2, label %loop.exit, label %loop.interior
loop.exit:
%X3.lcssa = phi(X3)
@llvm.dbg.value(var = %X3.lcssa) <---- Incorrect!
%X4 = %X3.lcssa + 3
In the pseudo-IR above, `insertDebugValuesForPHIs()` propogates the incorrect `dbg.value()` intrinsic. The intrinsic that should be propogated is the one associated with `%Y1` because that is the most recent assignment to `var`, but it is simply ignored. This results in incorrect debugging information as it is effectively re-ordering assignments.
This change removes the call to `insertDebugValuesForPHIs()` from LCSSA, preventing incorrect `dbg.value` intrinsics from being propagated. As you can see in the locstats output below (generated by building Clang 3.4 with `RelWithDebInfo` with and without my change), even though the number of variables in the 100% bucket has gone down, the number of variables in the 0% bucket has also gone down. Apart from the fact that coverage has increased, incorrect debug information has also been removed (as seen in the example above). This is because even though LCSSA no longer inserts debug intrinsics for new PHIs, not inserting an intrinsic at all allows the last dominating dbg.value to continue dominating later instructions which results in the correct behaviour.
Before:
=================================================
Debug Location Statistics
=================================================
cov% samples percentage(~)
-------------------------------------------------
0% 841882 25%
(0%,10%) 41547 1%
[10%,20%) 49777 1%
[20%,30%) 49478 1%
[30%,40%) 43095 1%
[40%,50%) 45196 1%
[50%,60%) 56624 1%
[60%,70%) 53013 1%
[70%,80%) 65112 1%
[80%,90%) 71479 2%
[90%,100%) 100112 2%
100% 1935854 57%
=================================================
-the number of debug variables processed: 3353169
-PC ranges covered: 56%
-------------------------------------------------
-total availability: 61%
=================================================
After:
=================================================
Debug Location Statistics
=================================================
cov% samples percentage(~)
-------------------------------------------------
0% 841874 25%
(0%,10%) 41652 1%
[10%,20%) 49847 1%
[20%,30%) 49592 1%
[30%,40%) 43230 1%
[40%,50%) 45257 1%
[50%,60%) 56594 1%
[60%,70%) 52978 1%
[70%,80%) 65097 1%
[80%,90%) 71391 2%
[90%,100%) 99888 2%
100% 1935817 57%
=================================================
-the number of debug variables processed: 3353217
-PC ranges covered: 56%
-------------------------------------------------
-total availability: 61%
=================================================
(I've analyzed samples of both increases and decreases and confirmed that these changes are caused by my change in LCSSA and that the changes result in the correct behaviour)
https://reviews.llvm.org/D92576
Files:
llvm/lib/Transforms/Utils/LCSSA.cpp
llvm/test/Transforms/LCSSA/DontInsertDebugValuesForPHIs.ll
llvm/test/Transforms/LCSSA/basictest.ll
Index: llvm/test/Transforms/LCSSA/basictest.ll
===================================================================
--- llvm/test/Transforms/LCSSA/basictest.ll
+++ llvm/test/Transforms/LCSSA/basictest.ll
@@ -20,7 +20,6 @@
loop.exit: ; preds = %post.if
; CHECK: %X3.lcssa = phi i32
; DEBUGIFY: %X3.lcssa = phi i32 {{.*}}, !dbg ![[DbgLoc:[0-9]+]]
-; DEBUGIFY-NEXT: call void @llvm.dbg.value(metadata i32 %X3.lcssa
; CHECK: %X4 = add i32 3, %X3.lcssa
%X4 = add i32 3, %X3 ; <i32> [#uses=0]
ret void
Index: llvm/test/Transforms/LCSSA/DontInsertDebugValuesForPHIs.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/LCSSA/DontInsertDebugValuesForPHIs.ll
@@ -0,0 +1,57 @@
+; RUN: opt < %s -lcssa -S | FileCheck %s
+
+; This test ensures that LCSSA does not insert dbg.value intrinsics using
+; insertDebugValuesForPHIs() which effectively cause assignments to be
+; re-ordered.
+; See PR48206 for more information.
+
+define dso_local i32 @_Z5lcssab(i1 zeroext %S2) {
+entry:
+ br label %loop.interior
+
+loop.interior: ; preds = %post.if, %entry
+ br i1 %S2, label %if.true, label %if.false
+
+if.true: ; preds = %loop.interior
+ %X1 = add i32 0, 0
+ br label %post.if
+
+if.false: ; preds = %loop.interior
+ %X2 = add i32 0, 1
+ br label %post.if
+
+post.if: ; preds = %if.false, %if.true
+ %X3 = phi i32 [ %X1, %if.true ], [ %X2, %if.false ], !dbg !21
+ call void @llvm.dbg.value(metadata i32 %X3, metadata !9, metadata !DIExpression()), !dbg !21
+ %Y1 = add i32 4, %X3, !dbg !22
+ call void @llvm.dbg.value(metadata i32 %Y1, metadata !9, metadata !DIExpression()), !dbg !22
+ br i1 %S2, label %loop.exit, label %loop.interior, !dbg !23
+
+loop.exit: ; preds = %post.if
+; CHECK: loop.exit:
+; CHECK-NEXT: %X3.lcssa = phi i32
+; CHECK-NOT: call void @llvm.dbg.value
+ %X4 = add i32 3, %X3
+ ret i32 %X4
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.debugify = !{!3, !4}
+!llvm.module.flags = !{!5}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify and Author", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "./testcase.ll", directory: "/")
+!2 = !{}
+!3 = !{i32 11}
+!4 = !{i32 5}
+!5 = !{i32 2, !"Debug Info Version", i32 3}
+!6 = distinct !DISubprogram(name: "_Z5lcssab", linkageName: "_Z5lcssab", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !{!9})
+!7 = !DISubroutineType(types: !2)
+!9 = !DILocalVariable(name: "var", scope: !6, file: !1, line: 3, type: !10)
+!10 = !DIBasicType(name: "ty32", size: 32, encoding: DW_ATE_unsigned)
+!21 = !DILocation(line: 7, column: 1, scope: !6)
+!22 = !DILocation(line: 8, column: 1, scope: !6)
+!23 = !DILocation(line: 9, column: 1, scope: !6)
+
Index: llvm/lib/Transforms/Utils/LCSSA.cpp
===================================================================
--- llvm/lib/Transforms/Utils/LCSSA.cpp
+++ llvm/lib/Transforms/Utils/LCSSA.cpp
@@ -265,15 +265,11 @@
Worklist.push_back(PostProcessPN);
// Keep track of PHI nodes that we want to remove because they did not have
- // any uses rewritten. If the new PHI is used, store it so that we can
- // try to propagate dbg.value intrinsics to it.
- SmallVector<PHINode *, 2> NeedDbgValues;
+ // any uses rewritten.
for (PHINode *PN : AddedPHIs)
if (PN->use_empty())
LocalPHIsToRemove.insert(PN);
- else
- NeedDbgValues.push_back(PN);
- insertDebugValuesForPHIs(InstBB, NeedDbgValues);
+
Changed = true;
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D92576.309193.patch
Type: text/x-patch
Size: 3859 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201203/b166cb4c/attachment.bin>
More information about the llvm-commits
mailing list