Support using sample profiles with partial debug info.

[dnovillo]

Summary

When using a profile, we used to require the use -gmlt so that we could
get access to the line locations. This is used to match line numbers in
the input profile to the line numbers in the function's IR.

But this is actually not necessary. The driver can provide source
location tracking without the emission of debug information. In these
cases, the annotation 'llvm.dbg.cu' is missing from the IR, but the
actual line location annotations are still present.

This patch adds a new way of looking for the start of the current
function. Instead of looking through the compile units in llvm.dbg.cu,
we can walk up the scope for the first instruction in the function with
a debug loc. If that describes the function, we use it. Otherwise, we
keep looking until we find one.

If no such instruction is found, we then give up and produce a warning.
I changed the diagnostic from an error to a warning because it's not
really a codegen problem. The compiler can continue, it's just that the
optimization opportunities won't include profile information.

[echristo]

No need for the \p but otherwise it's probably ok. If you could put the actual code in the test that would be nice.

Thanks!

[dblaikie]

lib/Transforms/Scalar/SampleProfile.cpp

662 ↗

(On Diff #15199)

Please use range-based-for loops.

671 ↗

(On Diff #15199)

If you find an instruction with DebugLoc, you don't need to keep searching - no matter what its subprogram node is.

This is what I tried to describe with the pseudocode I mentioned on IRC the other day.

for (basic blocks)

for (instructions)
  if (debugloc.isvalid)
    if (getsubprogram(debugloc).describes(F))
      return subprogram.getLineNumber()
    else
      /* break out of all the loops & consider this a failure */

(maybe wrap all this in a function so you can more easily early-exit when you reach that situation)

Here's the invariant that I believe now holds:
If a function has debug info, the scope chain of all instructions in that function will lead to the function and nothing else.

So once you find one instruction with a debug loc, you don't need to examine any others - if it leads to this function, you're done, if it doesn't then this function doesn't have debug info.

test/Transforms/SampleProfile/loc-tracking-only.ll

3 ↗

(On Diff #15199)

Worth updating calls.ll itself instead of adding a new test? If you don't depend on the llvm.dbg.cu at all, it doesn't seem worthwhile to have two different tests, one with it and one without it.

[dnovillo]

• Re-factor DISubprogram locator code.
• Remove unecessary test.

[dblaikie]

lib/Transforms/Scalar/SampleProfile.cpp

647 ↗

(On Diff #15215)

Why do we need this code at all? Should we just remove it in favor of the other code?

[echristo]

I'd have thought it'd be faster too.

[dnovillo]

• Do not try to use llvm.dbg.cu to find the subprogram for F.

[dblaikie]

Some optional tidbits, but otherwise fine.

lib/Transforms/Scalar/SampleProfile.cpp

646 ↗

(On Diff #15219)

usually we do this with an early continue to avoid extra indentation

if (DLoc.isKnown())
  continue;
/* more stuff */

& yeah, no worries about the range-for loops, a single cleanup would be fine

675 ↗

(On Diff #15219)

Separating the change in text and error->warning (& the subsequent test changes) into a separate commit would be nice.

[dnovillo]

Closed by commit rL220382 (authored by @dnovillo).

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

if (!AllWeightsZero) {
       TI->setMetadata(llvm::LLVMContext::MD_prof,
                       MDB.createBranchWeights(Weights));
     } else {
       DEBUG(dbgs() << "SKIPPED. All branch weights are zero.\n");
     }
   }
 }
 
+/// \brief Locate the DISubprogram for F.
+///
+/// We look for the first instruction that has a debug annotation
+/// leading back to \p F.
+///
+/// \returns a valid DISubprogram, if found. Otherwise, it returns an empty
+/// DISubprogram.
+static const DISubprogram getDISubprogram(Function &F, const LLVMContext &Ctx) {
+  for (Function::iterator I = F.begin(), E = F.end(); I != E; ++I) {
+    BasicBlock *B = I;
+    for (BasicBlock::iterator BI = B->begin(), BE = B->end(); BI != BE; ++BI) {
+      Instruction &Inst = *BI;
+      DebugLoc DLoc = Inst.getDebugLoc();
+      if (DLoc.isUnknown())
+        continue;
+      const MDNode *Scope = DLoc.getScopeNode(Ctx);
+      DISubprogram Subprogram = getDISubprogram(Scope);
+      return Subprogram.describes(&F) ? Subprogram : DISubprogram();
+    }
+  }
+
+  return DISubprogram();
+}
+
 /// \brief Get the line number for the function header.
 ///
 /// This looks up function \p F in the current compilation unit and
 /// retrieves the line number where the function is defined. This is
 /// line 0 for all the samples read from the profile file. Every line
 /// number is relative to this line.
 ///
 /// \param F  Function object to query.
 ///
 /// \returns the line number where \p F is defined. If it returns 0,
 ///          it means that there is no debug information available for \p F.
 unsigned SampleProfileLoader::getFunctionLoc(Function &F) {
-  NamedMDNode *CUNodes = F.getParent()->getNamedMetadata("llvm.dbg.cu");
-  if (CUNodes) {
-    for (unsigned I = 0, E1 = CUNodes->getNumOperands(); I != E1; ++I) {
-      DICompileUnit CU(CUNodes->getOperand(I));
-      DIArray Subprograms = CU.getSubprograms();
-      for (unsigned J = 0, E2 = Subprograms.getNumElements(); J != E2; ++J) {
-        DISubprogram Subprogram(Subprograms.getElement(J));
-        if (Subprogram.describes(&F))
-          return Subprogram.getLineNumber();
-      }
-    }
-  }
+  const DISubprogram &S = getDISubprogram(F, *Ctx);
+  if (S.isSubprogram())
+    return S.getLineNumber();
 
+  // If could not find the start of \p F, emit a diagnostic to inform the user
+  // about the missed opportunity.
   F.getContext().diagnose(DiagnosticInfoSampleProfile(
       "No debug information found in function " + F.getName()));
   return 0;
 }
 
 /// \brief Generate branch weight metadata for all branches in \p F.
 ///
 /// Branch weights are computed out of instruction samples using a

llvm/trunk/test/Transforms/SampleProfile/calls.ll

 ;
 ; int main() {
 ;   int s, i = 0;
 ;   while (i++ < 20000 * 20000)
 ;     if (i != 100) s = sum(i, s); else s = 30;
 ;   printf("sum is %d\n", s);
 ;   return 0;
 ; }
-
+;
+; Note that this test is missing the llvm.dbg.cu annotation. This emulates
+; the effect of the user having only used -fprofile-sample-use without
+; -gmlt when invoking the driver. In those cases, we need to track source
+; location information but we do not have to generate debug info in the
+; final binary.
 @.str = private unnamed_addr constant [11 x i8] c"sum is %d\0A\00", align 1
 
 ; Function Attrs: nounwind uwtable
 define i32 @_Z3sumii(i32 %x, i32 %y) {
 entry:
   %x.addr = alloca i32, align 4
   %y.addr = alloca i32, align 4
   store i32 %x, i32* %x.addr, align 4

 while.end:                                        ; preds = %while.cond
   %4 = load i32* %s, align 4, !dbg !24
   %call2 = call i32 (i8*, ...)* @printf(i8* getelementptr inbounds ([11 x i8]* @.str, i32 0, i32 0), i32 %4), !dbg !24
   ret i32 0, !dbg !25
 }
 
 declare i32 @printf(i8*, ...) #2
 
-!llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!8, !9}
 !llvm.ident = !{!10}
 
 !0 = metadata !{metadata !"0x11\004\00clang version 3.5 \000\00\000\00\000", metadata !1, metadata !2, metadata !2, metadata !3, metadata !2, metadata !2} ; [ DW_TAG_compile_unit ] [./calls.cc] [DW_LANG_C_plus_plus]
 !1 = metadata !{metadata !"calls.cc", metadata !"."}
 !2 = metadata !{}
 !3 = metadata !{metadata !4, metadata !7}
 !4 = metadata !{metadata !"0x2e\00sum\00sum\00\003\000\001\000\006\00256\000\003", metadata !1, metadata !5, metadata !6, null, i32 (i32, i32)* @_Z3sumii, null, null, metadata !2} ; [ DW_TAG_subprogram ] [line 3] [def] [sum]