[CallSiteSplitting] Don't let debug intrinsics affect optimizations

[uabelho]

Summary

This solves PR35616.

We don't want the compiler to generate different code when we compile
with/without -g, so we now ignore debug intrinsics when determining if
the optimization can trigger or not.

[uabelho]

Does the patch make sense?

I don't know this code at all or if there are other problems that also needs to be addressed if we allow dbg.values to be
between the phi nodes and the call.

[junbuml]

Thanks for catching this.
LGTM, assuming that the order of the call site and dbg doesn't matter as the dbg is placed after call-sites split after the transformation.

[uabelho]

In D41068#951333, @junbuml wrote:

Thanks for catching this.
LGTM, assuming that the order of the call site and dbg doesn't matter as the dbg is placed after call-sites split after the transformation.

Thanks! Yes that's my assumption as well.

llvm/trunk/lib/Transforms/Scalar/CallSiteSplitting.cpp

static bool canSplitCallSite(CallSite CS) {
   if (!isa<CallInst>(Instr))
     return false;
 
   // Allow splitting a call-site only when there is no instruction before the
   // call-site in the basic block. Based on this constraint, we only clone the
   // call instruction, and we do not move a call-site across any other
   // instruction.
   BasicBlock *CallSiteBB = Instr->getParent();
-  if (Instr != CallSiteBB->getFirstNonPHI())
+  if (Instr != CallSiteBB->getFirstNonPHIOrDbg())
     return false;
 
   // Need 2 predecessors and cannot split an edge from an IndirectBrInst.
   SmallVector<BasicBlock *, 2> Preds(predecessors(CallSiteBB));
   if (Preds.size() != 2 || isa<IndirectBrInst>(Preds[0]->getTerminator()) ||
       isa<IndirectBrInst>(Preds[1]->getTerminator()))
     return false;
 

 ///
 /// Note that for an OR predicated case, CallInst1 and CallInst2 should be
 /// created with more constrained arguments in
 /// createCallSitesOnOrPredicatedArgument().
 static void splitCallSite(CallSite CS, BasicBlock *PredBB1, BasicBlock *PredBB2,
                           Instruction *CallInst1, Instruction *CallInst2) {
   Instruction *Instr = CS.getInstruction();
   BasicBlock *TailBB = Instr->getParent();
-  assert(Instr == (TailBB->getFirstNonPHI()) && "Unexpected call-site");
+  assert(Instr == (TailBB->getFirstNonPHIOrDbg()) && "Unexpected call-site");
 
   BasicBlock *SplitBlock1 =
       SplitBlockPredecessors(TailBB, PredBB1, ".predBB1.split");
   BasicBlock *SplitBlock2 =
       SplitBlockPredecessors(TailBB, PredBB2, ".predBB2.split");
 
   assert((SplitBlock1 && SplitBlock2) && "Unexpected new basic block split.");
 
for (auto &CI : CS.args()) {
         CS2.setArgument(ArgNo, PN->getIncomingValueForBlock(SplitBlock2));
       }
       ++ArgNo;
     }
   }
 
   // Replace users of the original call with a PHI mering call-sites split.
   if (Instr->getNumUses()) {
-    PHINode *PN = PHINode::Create(Instr->getType(), 2, "phi.call", Instr);
+    PHINode *PN = PHINode::Create(Instr->getType(), 2, "phi.call",
+                                  TailBB->getFirstNonPHI());
     PN->addIncoming(CallInst1, SplitBlock1);
     PN->addIncoming(CallInst2, SplitBlock2);
     Instr->replaceAllUsesWith(PN);
   }
   DEBUG(dbgs() << "split call-site : " << *Instr << " into \n");
   DEBUG(dbgs() << "    " << *CallInst1 << " in " << SplitBlock1->getName()
                << "\n");
   DEBUG(dbgs() << "    " << *CallInst2 << " in " << SplitBlock2->getName()
                << "\n");
   Instr->eraseFromParent();
   NumCallSiteSplit++;
 }
 
 // Return true if the call-site has an argument which is a PHI with only
 // constant incoming values.
 static bool isPredicatedOnPHI(CallSite CS) {
   Instruction *Instr = CS.getInstruction();
   BasicBlock *Parent = Instr->getParent();
-  if (Instr != Parent->getFirstNonPHI())
+  if (Instr != Parent->getFirstNonPHIOrDbg())
     return false;
 
   for (auto &BI : *Parent) {
     if (PHINode *PN = dyn_cast<PHINode>(&BI)) {
       for (auto &I : CS.args())
         if (&*I == PN) {
           assert(PN->getNumIncomingValues() == 2 &&
                  "Unexpected number of incoming values");

llvm/trunk/test/Transforms/CallSiteSplitting/callsite-split-debug.ll

+; RUN: opt -S -callsite-splitting -o - < %s | FileCheck %s
+; RUN: opt -S -strip-debug -callsite-splitting -o - < %s | FileCheck %s
+
+define internal i16 @bar(i16 %p1, i16 %p2) {
+  %_tmp3 = mul i16 %p2, %p1
+  ret i16 %_tmp3
+}
+
+define i16 @foo(i16 %in) {
+bb0:
+  br label %bb1
+
+bb1:
+  %0 = icmp ne i16 %in, 0
+  br i1 %0, label %bb2, label %CallsiteBB
+
+bb2:
+  br label %CallsiteBB
+
+CallsiteBB:
+  %1 = phi i16 [ 0, %bb1 ], [ 1, %bb2 ]
+  %c = phi i16 [ 2, %bb1 ], [ 3, %bb2 ]
+  call void @llvm.dbg.value(metadata i16 %c, metadata !7, metadata !DIExpression()), !dbg !8
+  %2 = call i16 @bar(i16 %1, i16 5)
+  ret i16 %2
+}
+
+; Function Attrs: nounwind readnone speculatable
+declare void @llvm.dbg.value(metadata, metadata, metadata) #0
+
+attributes #0 = { nounwind readnone speculatable }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3}
+!llvm.ident = !{!4}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "My Compiler")
+!1 = !DIFile(filename: "foo.c", directory: "/bar")
+!2 = !{i32 2, !"Dwarf Version", i32 4}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{!"My Compiler"}
+!5 = !DIBasicType(name: "int", size: 16, encoding: DW_ATE_signed)
+!6 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 4, unit: !0)
+!7 = !DILocalVariable(name: "c", scope: !6, line: 5, type: !5)
+!8 = !DILocation(line: 5, column: 7, scope: !6)
+
+; The optimization should trigger even in the presence of the dbg.value in
+; CallSiteBB.
+
+; CHECK-LABEL: @foo
+; CHECK-LABEL: CallsiteBB.predBB1.split:
+; CHECK: [[TMP1:%[0-9]+]] = call i16 @bar(i16 1, i16 5)
+; CHECK-LABEL: CallsiteBB.predBB2.split:
+; CHECK: [[TMP2:%[0-9]+]] = call i16 @bar(i16 0, i16 5)
+; CHECK-LABEL: CallsiteBB
+; CHECK: %phi.call = phi i16 [ [[TMP1]], %CallsiteBB.predBB1.split ], [ [[TMP2]], %CallsiteBB.predBB2.split
+