[BasicBlockUtils] Set debug locations for instructions created in SplitBlockPredecessors.

[dblaikie]

Your test only tests 2 branch instructions, yet the code change touches 3 different sites - is one of those changes untested?

[samsonov]

• Extend test to splitting landingpad blocks.

[samsonov]

In D10343#185735, @dblaikie wrote:

Your test only tests 2 branch instructions, yet the code change touches 3 different sites - is one of those changes untested?

Right. Extended the test to cover all the cases.

[dblaikie]

Looks good to me - thanks!

(if the test case is derived from some C/C++ source, you could include that source (& the command line - optimization flags, etc) used to produce it in a comment in the source - though I realize this sort of test just needs debug locations so it's more managable by hand now that we have legible debug info metadata)

[samsonov]

Thanks for the review! The test case is loosely derived from C++ source, but heavily minimized and changed after that, so I'd probably just leave it as is.

lib/Transforms/Utils/BasicBlockUtils.cpp

if (BB->isLandingPad()) {
     std::string NewName = std::string(Suffix) + ".split-lp";
 
     SplitLandingPadPredecessors(BB, Preds, Suffix, NewName.c_str(),
                                 NewBBs, AA, DT, LI, PreserveLCSSA);
     return NewBBs[0];
   }
 
   // Create new basic block, insert right before the original block.
-  BasicBlock *NewBB = BasicBlock::Create(BB->getContext(), BB->getName()+Suffix,
-                                         BB->getParent(), BB);
+  BasicBlock *NewBB = BasicBlock::Create(
+      BB->getContext(), BB->getName() + Suffix, BB->getParent(), BB);
 
   // The new block unconditionally branches to the old block.
   BranchInst *BI = BranchInst::Create(BB, NewBB);
+  BI->setDebugLoc(BB->getFirstNonPHI()->getDebugLoc());
 
   // Move the edges from Preds to point to NewBB instead of BB.
   for (unsigned i = 0, e = Preds.size(); i != e; ++i) {
     // This is slightly more strict than necessary; the minimum requirement
     // is that there be no more than one indirectbr branching to BB. And
     // all BlockAddress uses would need to be updated.
     assert(!isa<IndirectBrInst>(Preds[i]->getTerminator()) &&
            "Cannot split an edge from an IndirectBrInst");
void llvm::SplitLandingPadPredecessors(BasicBlock *OrigBB,
   // it right before the original block.
   BasicBlock *NewBB1 = BasicBlock::Create(OrigBB->getContext(),
                                           OrigBB->getName() + Suffix1,
                                           OrigBB->getParent(), OrigBB);
   NewBBs.push_back(NewBB1);
 
   // The new block unconditionally branches to the old block.
   BranchInst *BI1 = BranchInst::Create(OrigBB, NewBB1);
+  BI1->setDebugLoc(OrigBB->getFirstNonPHI()->getDebugLoc());
 
   // Move the edges from Preds to point to NewBB1 instead of OrigBB.
   for (unsigned i = 0, e = Preds.size(); i != e; ++i) {
     // This is slightly more strict than necessary; the minimum requirement
     // is that there be no more than one indirectbr branching to BB. And
     // all BlockAddress uses would need to be updated.
     assert(!isa<IndirectBrInst>(Preds[i]->getTerminator()) &&
            "Cannot split an edge from an IndirectBrInst");
if (!NewBB2Preds.empty()) {
     // Create another basic block for the rest of OrigBB's predecessors.
     NewBB2 = BasicBlock::Create(OrigBB->getContext(),
                                 OrigBB->getName() + Suffix2,
                                 OrigBB->getParent(), OrigBB);
     NewBBs.push_back(NewBB2);
 
     // The new block unconditionally branches to the old block.
     BranchInst *BI2 = BranchInst::Create(OrigBB, NewBB2);
+    BI2->setDebugLoc(OrigBB->getFirstNonPHI()->getDebugLoc());
 
     // Move the remaining edges from OrigBB to point to NewBB2.
     for (SmallVectorImpl<BasicBlock*>::iterator
            i = NewBB2Preds.begin(), e = NewBB2Preds.end(); i != e; ++i)
       (*i)->getTerminator()->replaceUsesOfWith(OrigBB, NewBB2);
 
     // Update DominatorTree, LoopInfo, and LCCSA analysis information.
     HasLoopExit = false;

lib/Transforms/Utils/LoopSimplify.cpp

for (pred_iterator PI = pred_begin(Header), PE = pred_end(Header);
     }
   }
 
   // Split out the loop pre-header.
   BasicBlock *PreheaderBB;
   PreheaderBB = SplitBlockPredecessors(Header, OutsideBlocks, ".preheader",
                                        AA, DT, LI, PreserveLCSSA);
 
-  PreheaderBB->getTerminator()->setDebugLoc(
-                                      Header->getFirstNonPHI()->getDebugLoc());
   DEBUG(dbgs() << "LoopSimplify: Creating pre-header "
                << PreheaderBB->getName() << "\n");
 
   // Make sure that NewBB is put someplace intelligent, which doesn't mess up
   // code layout too horribly.
   placeSplitBlockCarefully(PreheaderBB, OutsideBlocks, L);
 
   return PreheaderBB;

test/Transforms/LoopSimplify/dbg-loc.ll

+; Check that LoopSimplify creates debug locations in synthesized basic blocks.
+; RUN: opt -loop-simplify %s -S -o - | FileCheck %s
+
+%union.anon = type { i32 }
+%"Length" = type <{ %union.anon, i8, i8, i8, i8 }>
+declare void @bar(%"Length"*) #3
+@catchtypeinfo = external unnamed_addr constant { i8*, i8*, i8* }
+declare i32 @__gxx_personality_v0(...)
+declare void @f1()
+declare void @f2()
+declare void @f3()
+
+; CHECK-LABEL: @foo
+; CHECK:       for.body.preheader:
+; CHECK-NEXT:    br label %for.body, !dbg [[PREHEADER_LOC:![0-9]+]]
+; CHECK:       for.end.loopexit:
+; CHECK-NEXT:    br label %for.end, !dbg [[LOOPEXIT_LOC:![0-9]+]]
+
+define linkonce_odr hidden void @foo(%"Length"* %begin, %"Length"* %end) nounwind ssp uwtable align 2 {
+entry:
+  %cmp.4 = icmp eq %"Length"* %begin, %end, !dbg !7
+  br i1 %cmp.4, label %for.end, label %for.body, !dbg !8
+
+for.body:                                         ; preds = %entry, %length.exit
+  %begin.sink5 = phi %"Length"* [ %incdec.ptr, %length.exit ], [ %begin, %entry ]
+  %m_type.i.i.i = getelementptr inbounds %"Length", %"Length"* %begin.sink5, i64 0, i32 2, !dbg !9
+  %0 = load i8, i8* %m_type.i.i.i, align 1, !dbg !9
+  %cmp.i.i = icmp eq i8 %0, 9, !dbg !7
+  br i1 %cmp.i.i, label %if.then.i, label %length.exit, !dbg !8
+
+if.then.i:                                        ; preds = %for.body
+  tail call void @bar(%"Length"* %begin.sink5) #7, !dbg !10
+  br label %length.exit, !dbg !10
+
+length.exit:                        ; preds = %for.body, %if.then.i
+  %incdec.ptr = getelementptr inbounds %"Length", %"Length"* %begin.sink5, i64 1, !dbg !11
+  %cmp = icmp eq %"Length"* %incdec.ptr, %end, !dbg !7
+  br i1 %cmp, label %for.end, label %for.body, !dbg !8
+
+for.end:                                          ; preds = %length.exit, %entry
+  ret void, !dbg !12
+}
+
+; CHECK-LABEL: @with_landingpad
+; CHECK: catch.preheader:
+; CHECK:   br label %catch, !dbg [[LPAD_PREHEADER_LOC:![0-9]+]]
+; CHECK: catch.preheader.split-lp:
+; CHECK:   br label %catch, !dbg [[LPAD_PREHEADER_LOC]]
+
+define void @with_landingpad() uwtable ssp {
+entry:
+  invoke void @f1() to label %try.cont19 unwind label %catch, !dbg !13
+
+catch:                                            ; preds = %if.else, %entry
+  %0 = landingpad { i8*, i32 } personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*)
+          catch i8* bitcast ({ i8*, i8*, i8* }* @catchtypeinfo to i8*), !dbg !13
+  invoke void @f3() to label %if.else unwind label %eh.resume, !dbg !13
+
+if.else:                                          ; preds = %catch
+  invoke void @f2() to label %try.cont19 unwind label %catch, !dbg !13
+
+try.cont19:                                       ; preds = %if.else, %entry
+  ret void, !dbg !13
+
+eh.resume:                                        ; preds = %catch
+  %1 = landingpad { i8*, i32 } personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*)
+          cleanup catch i8* bitcast ({ i8*, i8*, i8* }* @catchtypeinfo to i8*), !dbg !13
+  resume { i8*, i32 } undef, !dbg !13
+}
+
+; CHECK-DAG: [[PREHEADER_LOC]] = !DILocation(line: 73, column: 27, scope: !{{[0-9]+}})
+; CHECK-DAG: [[LOOPEXIT_LOC]] = !DILocation(line: 75, column: 9, scope: !{{[0-9]+}})
+; CHECK-DAG: [[LPAD_PREHEADER_LOC]] = !DILocation(line: 85, column: 1, scope: !{{[0-9]+}})
+
+!llvm.module.flags = !{!0, !1, !2}
+!0 = !{i32 2, !"Dwarf Version", i32 4}
+!1 = !{i32 2, !"Debug Info Version", i32 3}
+!2 = !{i32 1, !"PIC Level", i32 2}
+
+!3 = !{}
+!4 = !DISubroutineType(types: !3)
+!5 = !DIFile(filename: "Vector.h", directory: "/tmp")
+!6 = !DISubprogram(name: "destruct", scope: !5, file: !5, line: 71, type: !4, isLocal: false, isDefinition: true, scopeLine: 72, flags: DIFlagPrototyped, isOptimized: false, function: void (%"Length"*, %"Length"*)* @foo, variables: !3)
+!7 = !DILocation(line: 73, column: 38, scope: !6)
+!8 = !DILocation(line: 73, column: 13, scope: !6)
+!9 = !DILocation(line: 73, column: 27, scope: !6)
+!10 = !DILocation(line: 74, column: 17, scope: !6)
+!11 = !DILocation(line: 73, column: 46, scope: !6)
+!12 = !DILocation(line: 75, column: 9, scope: !6)
+!13 = !DILocation(line: 85, column: 1, scope: !6)