[PseudoProbe] Clean up dwarf discriminator and avoid duplicating factor.

[hoy]

Summary

A pseudo probe is created with dwarf line information shared with its nearest instruction. If the instruction comes with a dwarf discriminator, it will be shared with the probe as well. This can confuse the later FS-AFDO discriminator assignment pass. To fix this, I'm cleaning up the discriminator fields for probes when they are inserted.

I also notice another possibility to change the discriminator field of pseudo probes in the pipeline before the FS discriminator assignment pass. That is the loop unroller, which assigns duplication factor to instruction being vectorized. I'm disabling that for pseudo probe intrinsics specifically, also for callsites with probes.

[wenlei]

llvm/include/llvm/IR/DebugInfoMetadata.h

2311

perhaps it's worth explaining in the comment, the model used by probe, and why duplication factor isn't needed.

2312–2313

since you already guard this at call sites, is this still needed, or should it be an assert?

line-based FS-AFDO doesn't use duplication factor either, where is the use of duplication factor prevented for that today?

llvm/lib/Transforms/IPO/SampleProfileProbe.cpp

356

What if two probes come from the same line? should we still expect a unique line+base_disc to be able to differentiate them?

[hoy]

llvm/include/llvm/IR/DebugInfoMetadata.h

2311

comment updated.

2312–2313

This is still needed as the guard on the callsite is to filter block probes. The logic here is mainly for callsite probes.

line-based FS-AFDO doesn't use duplication factor either, where is the use of duplication factor prevented for that today?

The callsites of this function are disabled at For FS-AFDO, e.g, https://github.com/llvm/llvm-project/blob/5362a0d859d8e96b3f7c0437b7866e17a818a4f7/llvm/lib/Transforms/Utils/LoopUnroll.cpp#L516 . The logic here is not only for FS-AFDO, but also for in general not screwing up callsite probe ids.

llvm/lib/Transforms/IPO/SampleProfileProbe.cpp

356

We do not need a unique line number for each probe as the probe id is used to determine the probe's discriminator.

[hoy]

Addressing comments.

[wenlei]

llvm/include/llvm/IR/DebugInfoMetadata.h

2312–2313

This is still needed as the guard on the callsite is to filter block probes. The logic here is mainly for callsite probes.

Where is the call site for call probes? Why this is guarded in the function for call probe, but at call site for block probe?

I saw three places where this clone is used, and two of the three you guarded them at call site. The remaining one is in VPlan.cpp. Is that inconsistency intentional?

llvm/lib/Transforms/IPO/SampleProfileProbe.cpp

356

Sounds good, hope it (not having unique line+disc) will not make merging probes more likely to happen for downstream opts..

[hoy]

llvm/include/llvm/IR/DebugInfoMetadata.h

2312–2313

Block probe is in form of probe intrinsics, and the checks enforced in this change, i.,e, if (!I.isDebugOrPseudoInst()), excludes probe intrinsics from getting duplicated factor updated.

Call probe is just attached to user call instruction in form of dwarf discriminator, and they are not excluded at the callsites of this function. We could do that, but I think it's more cleaner to do it here.

I saw three places where this clone is used, and two of the three you guarded them at call site. The remaining one is in VPlan.cpp. Is that inconsistency intentional?

Good catch. Should also exclude probe intrinsic there for completeness, though it doesn't affect anything since FS-AFDO is already excluded there.

llvm/lib/Transforms/IPO/SampleProfileProbe.cpp

356

Right, it will unlikely affect the merging which doesn't look at debug data anyways.

[hoy]

Updating D148569: [PseudoProbe] Clean up dwarf discriminator and avoid duplicating factor.

[hoy]

llvm/include/llvm/IR/DebugInfoMetadata.h

2312–2313

Actually we have to exclude probe there, otherwise the duplication factor will be mistakenly encoded as a FS-discriminator.

[wenlei]

lgtm, thanks.

[hoy]

Cleaning up a test.

llvm/include/llvm/IR/DebugInfoMetadata.h

DILocation::cloneWithBaseDiscriminator(unsigned D) const {
   if (std::optional<unsigned> Encoded = encodeDiscriminator(D, DF, CI))
     return cloneWithDiscriminator(*Encoded);
   return std::nullopt;
 }
 
 std::optional<const DILocation *>
 DILocation::cloneByMultiplyingDuplicationFactor(unsigned DF) const {
   assert(!EnableFSDiscriminator && "FSDiscriminator should not call this.");
+  // Do no interfere with pseudo probes. Pseudo probe doesn't need duplication
+  // factor support as samples collected on cloned probes will be aggregated.
+  // Also pseudo probe at a callsite uses the dwarf discriminator to store
+  // pseudo probe related information, such as the probe id.
+  if (isPseudoProbeDiscriminator(getDiscriminator()))
+    return this;
 
   DF *= getDuplicationFactor();
   if (DF <= 1)
     return this;
 
   unsigned BD = getBaseDiscriminator();
   unsigned CI = getCopyIdentifier();
   if (std::optional<unsigned> D = encodeDiscriminator(BD, DF, CI))

llvm/lib/Transforms/IPO/SampleProfileProbe.cpp

assert(Builder.GetInsertPoint() != BB->end() &&
            "Cannot get the probing point");
     Function *ProbeFn =
         llvm::Intrinsic::getDeclaration(M, Intrinsic::pseudoprobe);
     Value *Args[] = {Builder.getInt64(Guid), Builder.getInt64(Index),
                      Builder.getInt32(0),
                      Builder.getInt64(PseudoProbeFullDistributionFactor)};
     auto *Probe = Builder.CreateCall(ProbeFn, Args);
     AssignDebugLoc(Probe);
+    // Reset the dwarf discriminator if the debug location comes with any. The
+    // discriminator field may be used by FS-AFDO later in the pipeline.
+    if (auto DIL = Probe->getDebugLoc()) {
+      if (DIL->getDiscriminator()) {
+        DIL = DIL->cloneWithDiscriminator(0);
+        Probe->setDebugLoc(DIL);
+      }
+    }
   }
 
   // Probe both direct calls and indirect calls. Direct calls are probed so that
   // their probe ID can be used as an call site identifier to represent a
   // calling context.
   for (auto &I : CallProbeIds) {
     auto *Call = I.first;
     uint32_t Index = I.second;
     uint32_t Type = cast<CallBase>(Call)->getCalledFunction()
                         ? (uint32_t)PseudoProbeType::DirectCall
                         : (uint32_t)PseudoProbeType::IndirectCall;
     AssignDebugLoc(Call);
-    // Levarge the 32-bit discriminator field of debug data to store the ID and
-    // type of a callsite probe. This gets rid of the dependency on plumbing a
-    // customized metadata through the codegen pipeline.
-    uint32_t V = PseudoProbeDwarfDiscriminator::packProbeData(
-        Index, Type, 0, PseudoProbeDwarfDiscriminator::FullDistributionFactor);
     if (auto DIL = Call->getDebugLoc()) {
+      // Levarge the 32-bit discriminator field of debug data to store the ID
+      // and type of a callsite probe. This gets rid of the dependency on
+      // plumbing a customized metadata through the codegen pipeline.
+      uint32_t V = PseudoProbeDwarfDiscriminator::packProbeData(
+          Index, Type, 0,
+          PseudoProbeDwarfDiscriminator::FullDistributionFactor);
       DIL = DIL->cloneWithDiscriminator(V);
       Call->setDebugLoc(DIL);
     }
   }
 
   // Create module-level metadata that contains function info necessary to
   // synthesize probe-based sample counts,  which are
   // - FunctionGUID

llvm/lib/Transforms/Utils/LoopUnroll.cpp

for (Loop *SubLoop : *L)
     LoopsToSimplify.insert(SubLoop);
 
   // When a FSDiscriminator is enabled, we don't need to add the multiply
   // factors to the discriminators.
   if (Header->getParent()->shouldEmitDebugInfoForProfiling() &&
       !EnableFSDiscriminator)
     for (BasicBlock *BB : L->getBlocks())
       for (Instruction &I : *BB)
-        if (!isa<DbgInfoIntrinsic>(&I))
+        if (!I.isDebugOrPseudoInst())
           if (const DILocation *DIL = I.getDebugLoc()) {
             auto NewDIL = DIL->cloneByMultiplyingDuplicationFactor(ULO.Count);
             if (NewDIL)
               I.setDebugLoc(*NewDIL);
             else
               LLVM_DEBUG(dbgs()
                          << "Failed to create new discriminator: "
                          << DIL->getFilename() << " Line: " << DIL->getLine());

llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp

llvm::UnrollAndJamLoop(Loop *L, unsigned Count, unsigned TripCount,
   LoopBlocksDFS::RPOIterator BlockEnd = DFS.endRPO();
 
   // When a FSDiscriminator is enabled, we don't need to add the multiply
   // factors to the discriminators.
   if (Header->getParent()->shouldEmitDebugInfoForProfiling() &&
       !EnableFSDiscriminator)
     for (BasicBlock *BB : L->getBlocks())
       for (Instruction &I : *BB)
-        if (!isa<DbgInfoIntrinsic>(&I))
+        if (!I.isDebugOrPseudoInst())
           if (const DILocation *DIL = I.getDebugLoc()) {
             auto NewDIL = DIL->cloneByMultiplyingDuplicationFactor(Count);
             if (NewDIL)
               I.setDebugLoc(*NewDIL);
             else
               LLVM_DEBUG(dbgs()
                          << "Failed to create new discriminator: "
                          << DIL->getFilename() << " Line: " << DIL->getLine());

llvm/lib/Transforms/Vectorize/VPlan.cpp

if (!Inst) {
     Builder.SetCurrentDebugLocation(DebugLoc());
     return;
   }
 
   const DILocation *DIL = Inst->getDebugLoc();
   // When a FSDiscriminator is enabled, we don't need to add the multiply
   // factors to the discriminators.
   if (DIL && Inst->getFunction()->shouldEmitDebugInfoForProfiling() &&
-      !isa<DbgInfoIntrinsic>(Inst) && !EnableFSDiscriminator) {
+      !Inst->isDebugOrPseudoInst() && !EnableFSDiscriminator) {
     // FIXME: For scalable vectors, assume vscale=1.
     auto NewDIL =
         DIL->cloneByMultiplyingDuplicationFactor(UF * VF.getKnownMinValue());
     if (NewDIL)
       Builder.SetCurrentDebugLocation(*NewDIL);
     else
       LLVM_DEBUG(dbgs() << "Failed to create new discriminator: "
                         << DIL->getFilename() << " Line: " << DIL->getLine());

llvm/test/Transforms/LoopVectorize/discriminator.ll

 ; RUN: opt -S -passes=loop-vectorize -force-vector-width=4 -force-vector-interleave=1 < %s | FileCheck --check-prefix=DBG_VALUE --check-prefix=LOOPVEC_4_1 %s
 ; RUN: opt -S -passes=loop-vectorize -force-vector-width=2 -force-vector-interleave=3 < %s | FileCheck --check-prefix=DBG_VALUE --check-prefix=LOOPVEC_2_3 %s
 ; RUN: opt -S -passes=loop-unroll  -unroll-count=5 < %s | FileCheck --check-prefix=DBG_VALUE --check-prefix=LOOPUNROLL_5 %s
 ; RUN: opt -S -passes=loop-vectorize,loop-unroll -force-vector-width=4 -force-vector-interleave=4 - -unroll-count=2 < %s | FileCheck --check-prefix=DBG_VALUE --check-prefix=LOOPVEC_UNROLL %s
+; RUN: opt -S -passes=pseudo-probe,loop-unroll  -unroll-count=5 < %s | FileCheck --check-prefix=PSEUDO_PROBE %s
 
 ; Test if vectorization/unroll factor is recorded in discriminator.
 ;
 ; Original source code:
 ;  1 int *a;
 ;  2 int *b;
 ;  3
 ;  4 void foo() {

 
 ; <label>:3:                                      ; preds = %3, %0
   %indvars.iv = phi i64 [ 0, %0 ], [ %indvars.iv.next, %3 ]
   %4 = getelementptr inbounds i32, ptr %1, i64 %indvars.iv, !dbg !8
   %5 = load i32, ptr %4, align 4, !dbg !8, !tbaa !15
   %6 = getelementptr inbounds i32, ptr %2, i64 %indvars.iv, !dbg !13
   %7 = load i32, ptr %6, align 4, !dbg !17, !tbaa !15
   %8 = add nsw i32 %7, %5, !dbg !17
+;PSEUDO_PROBE-COUNT-5: call void @llvm.pseudoprobe(i64 6699318081062747564, i64 2, i32 0, i64 -1), !dbg ![[#PROBE:]]
 ;DBG_VALUE: call void @llvm.dbg.declare{{.*}}!dbg ![[DBG:[0-9]*]]
   call void @llvm.dbg.declare(metadata i32 %8, metadata !22, metadata !DIExpression()), !dbg !17
   store i32 %8, ptr %6, align 4, !dbg !17, !tbaa !15
   %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1, !dbg !18
   %exitcond = icmp eq i64 %indvars.iv.next, 4096, !dbg !19
   br i1 %exitcond, label %9, label %3, !dbg !14, !llvm.loop !20
 
 ; <label>:9:                                      ; preds = %3
   ret void, !dbg !21
 }
 
 ;DBG_VALUE: ![[TOP:[0-9]*]] = distinct !DISubprogram(name: "foo"
 ;LOOPVEC_4_1: discriminator: 17
 ;LOOPVEC_2_3: discriminator: 25
 ;LOOPUNROLL_5: discriminator: 21
 ; When unrolling after loop vectorize, both vec_body and remainder loop
 ; are unrolled.
 ;LOOPVEC_UNROLL: discriminator: 9
 ;LOOPVEC_UNROLL: discriminator: 385
 ;DBG_VALUE: ![[DBG]] = {{.*}}, scope: ![[TOP]]
+; Pseudo probe should not have duplication factor assigned.
+;PSEUDO_PROBE: ![[TOP:[0-9]*]] = distinct !DISubprogram(name: "foo"
+;PSEUDO_PROBE: ![[#PROBE]] = !DILocation(line: 6, column: 13, scope: ![[TOP]])
 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!3, !4}
 
 !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, debugInfoForProfiling: true)
 !1 = !DIFile(filename: "a.cc", directory: "/")
 !3 = !{i32 2, !"Dwarf Version", i32 4}
 !4 = !{i32 2, !"Debug Info Version", i32 3}

llvm/test/Transforms/SampleProfile/pseudo-probe-emit.ll

; CHECK-ASM: .pseudoprobe [[#GUID2]] 3 2 0 foo2
   ret void
 }
 
 ; CHECK-IL: Function Attrs: nocallback nofree nosync nounwind willreturn memory(inaccessiblemem: readwrite)
 ; CHECK-IL-NEXT: declare void @llvm.pseudoprobe(i64, i64, i32, i64)
 
 ; CHECK-IL: ![[#FOO:]] = distinct !DISubprogram(name: "foo"
 ; CHECK-IL: ![[#FAKELINE]] = !DILocation(line: 0, scope: ![[#FOO]])
-; CHECK-IL: ![[#REALLINE]] = !DILocation(line: 2, scope: ![[#FOO]])
+; CHECK-IL: ![[#REALLINE]] = !DILocation(line: 2, scope: ![[#DISC0:]])
+; CHECK-IL: ![[#DISC0]] = !DILexicalBlockFile(scope: ![[#FOO]], file: ![[#]], discriminator: 0)
 ; CHECK-IL: ![[#PROBE0]] = !DILocation(line: 2, column: 20, scope: ![[#SCOPE0:]])
 ;; A discriminator of 67108887 which is 0x7200017 in hexdecimal, stands for a direct call probe
 ;; with an index of 2.
 ; CHECK-IL: ![[#SCOPE0]] = !DILexicalBlockFile(scope: ![[#]], file: ![[#]], discriminator: 119537687)
 ; CHECK-IL: ![[#PROBE1]] = !DILocation(line: 0, scope: ![[#SCOPE1:]])
 ;; A discriminator of 186646559 which is 0xb20001f in hexdecimal, stands for a direct call probe
 ;; with an index of 3.
 ; CHECK-IL: ![[#SCOPE1]] = !DILexicalBlockFile(scope: ![[#]], file: ![[#]], discriminator: 186646559)

 !3 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 1, type: !5, unit: !0, retainedNodes: !2)
 !4 = distinct !DISubprogram(name: "foo2", scope: !1, file: !1, line: 2, type: !5, unit: !0, retainedNodes: !2)
 !5 = !DISubroutineType(types: !6)
 !6 = !{!7}
 !7 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
 !9 = !{i32 2, !"Dwarf Version", i32 4}
 !10 = !{i32 2, !"Debug Info Version", i32 3}
 !11 = !{!"clang version 3.9.0"}
-!12 = !DILocation(line: 2, scope: !3)
+!12 = !DILocation(line: 2, scope: !14)
 !13 = !DILocation(line: 2, column: 20, scope: !4)
+!14 = !DILexicalBlockFile(scope: !3, file: !1, discriminator: 1)

llvm/test/Transforms/SampleProfile/pseudo-probe-profile-mismatch.ll

 !55 = !DILocalVariable(name: "a", scope: !56, file: !3, line: 17, type: !6)
 !56 = distinct !DILexicalBlock(scope: !57, file: !3, line: 17, column: 5)
 !57 = distinct !DILexicalBlock(scope: !58, file: !3, line: 16, column: 34)
 !58 = distinct !DILexicalBlock(scope: !54, file: !3, line: 16, column: 3)
 !59 = !DILocation(line: 16, column: 12, scope: !54)
 !60 = !DILocation(line: 0, scope: !54)
 !61 = !DILocation(line: 16, column: 8, scope: !54)
 !62 = !DILocation(line: 16, column: 19, scope: !63)
-!63 = !DILexicalBlockFile(scope: !58, file: !3, discriminator: 2)
+!63 = !DILexicalBlockFile(scope: !58, file: !3, discriminator: 0)
 !64 = !DILocation(line: 16, column: 21, scope: !63)
 !65 = !DILocation(line: 16, column: 3, scope: !66)
-!66 = !DILexicalBlockFile(scope: !54, file: !3, discriminator: 2)
+!66 = !DILexicalBlockFile(scope: !54, file: !3, discriminator: 0)
 !67 = !DILocation(line: 0, scope: !49)
 !68 = !DILocation(line: 23, column: 1, scope: !49)
 !69 = !DILocation(line: 17, column: 14, scope: !56)
 !70 = !DILocation(line: 0, scope: !56)
 !71 = !DILocation(line: 17, column: 10, scope: !56)
 !72 = !DILocation(line: 17, column: 21, scope: !73)
-!73 = !DILexicalBlockFile(scope: !74, file: !3, discriminator: 2)
+!73 = !DILexicalBlockFile(scope: !74, file: !3, discriminator: 0)
 !74 = distinct !DILexicalBlock(scope: !56, file: !3, line: 17, column: 5)
 !75 = !DILocation(line: 17, column: 23, scope: !73)
 !76 = !DILocation(line: 17, column: 5, scope: !77)
-!77 = !DILexicalBlockFile(scope: !56, file: !3, discriminator: 2)
+!77 = !DILexicalBlockFile(scope: !56, file: !3, discriminator: 0)
 !78 = !DILocation(line: 22, column: 3, scope: !57)
 !79 = !DILocation(line: 16, column: 30, scope: !80)
-!80 = !DILexicalBlockFile(scope: !58, file: !3, discriminator: 4)
+!80 = !DILexicalBlockFile(scope: !58, file: !3, discriminator: 0)
 !81 = !DILocation(line: 16, column: 3, scope: !80)
 !82 = distinct !{!82, !83, !84, !85}
 !83 = !DILocation(line: 16, column: 3, scope: !54)
 !84 = !DILocation(line: 22, column: 3, scope: !54)
 !85 = !{!"llvm.loop.mustprogress"}
 !86 = !DILocation(line: 18, column: 19, scope: !87)
 !87 = distinct !DILexicalBlock(scope: !74, file: !3, line: 17, column: 37)
 !88 = !DILocation(line: 18, column: 11, scope: !89)
 !89 = !DILexicalBlockFile(scope: !87, file: !3, discriminator: 186646631)
 !90 = !DILocation(line: 18, column: 9, scope: !87)
 !91 = !DILocation(line: 19, column: 15, scope: !87)
 !92 = !DILocation(line: 19, column: 11, scope: !93)
 !93 = !DILexicalBlockFile(scope: !87, file: !3, discriminator: 186646639)
 !94 = !DILocation(line: 19, column: 9, scope: !87)
 !95 = !DILocation(line: 20, column: 15, scope: !87)
 !96 = !DILocation(line: 20, column: 11, scope: !97)
 !97 = !DILexicalBlockFile(scope: !87, file: !3, discriminator: 186646647)
 !98 = !DILocation(line: 20, column: 9, scope: !87)
 !99 = !DILocation(line: 17, column: 33, scope: !100)
-!100 = !DILexicalBlockFile(scope: !74, file: !3, discriminator: 4)
+!100 = !DILexicalBlockFile(scope: !74, file: !3, discriminator: 0)
 !101 = !DILocation(line: 17, column: 5, scope: !100)
 !102 = distinct !{!102, !103, !104, !85}
 !103 = !DILocation(line: 17, column: 5, scope: !56)
 !104 = !DILocation(line: 21, column: 5, scope: !56)