This is an archive of the discontinued LLVM Phabricator instance.

[BasicBlockUtils] Use getFirstNonPHIOrDbg to set debugloc for instructions created in SplitBlockPredecessors
ClosedPublic

Authored by twoh on Feb 11 2017, 1:12 PM.

Details

Summary

When setting debugloc for instructions created in SplitBlockPredecessors, current implementation copies debugloc from the first-non-phi instruction of the original basic block. However, if the first-non-phi instruction is a call for @llvm.dbg.value, the debugloc of the instruction may point the location outside of the block itself. For the example code of

 1 typedef struct _node_t {
 2   struct _node_t *next;
 3 } node_t;
 4
 5 extern node_t *root;
 6
 7 int foo() {
 8   node_t *node, *tmp;
 9   int ret = 0;
10
11   node = tmp = root->next;
12   while (node != root) {
13     while (node) {
14       tmp = node;
15       node = node->next;
16       ret++;
17     }
18   }
19
20   return ret;
21 }

, below is the basicblock corresponding to line 12 after Reassociate expressions pass:

while.cond:                                       ; preds = %while.cond2, %entry
  %node.0 = phi %struct._node_t* [ %1, %entry ], [ null, %while.cond2 ]
  %ret.0 = phi i32 [ 0, %entry ], [ %ret.1, %while.cond2 ]
  tail call void @llvm.dbg.value(metadata i32 %ret.0, i64 0, metadata !19, metadata !20), !dbg !21
  tail call void @llvm.dbg.value(metadata %struct._node_t* %node.0, i64 0, metadata !11, metadata !20), !dbg !31
  %cmp = icmp eq %struct._node_t* %node.0, %0, !dbg !33
  br i1 %cmp, label %while.end5, label %while.cond2, !dbg !35

As you can see, the first-non-phi instruction is a call for @llvm.dbg.value, and the debugloc is

!21 = !DILocation(line: 9, column: 7, scope: !6)

, which is a definition of 'ret' variable and outside of the scope of the basicblock itself. However, current implementation picks up this debugloc for the instructions created in SplitBlockPredecessors. This patch addresses this problem by picking up debugloc from the first-non-phi-non-dbg instruction.

Diff Detail

Repository
rL LLVM

Event Timeline

twoh created this revision.Feb 11 2017, 1:12 PM
eugenis edited edge metadata.Feb 13 2017, 2:19 PM

Looks reasonable, but I wonder if the test case could be made simpler. Why not add an llvm.dbg.value call with some random debug location to the existing test case?

twoh updated this revision to Diff 88322.Feb 14 2017, 12:31 AM

Addressing @eugenis's comments. Thank you for the suggestion!

twoh updated this revision to Diff 88323.Feb 14 2017, 12:33 AM

nit: remove unnecessary #2 at the end of llvm.dbg.value declaration

eugenis accepted this revision.Feb 14 2017, 1:06 PM

Thanks, this LGTM.

This revision is now accepted and ready to land.Feb 14 2017, 1:06 PM
twoh added a comment.Feb 14 2017, 1:20 PM

Thanks for the review!

This revision was automatically updated to reflect the committed changes.