[FS-AFDO] Clean up non-zero discriminator for pseudo probes at the first FS discriminator pass.

[hoy]

Summary

The dwarf discriminator field for pseudo probes is not supposed to be used until the first FS discriminator pass. Unfortunately there are always corner cases that accidientally set this field. For example, the inliner could set this field for an inlined instruction if the instruction does not come with any debug information. While fixing all such spots is possible, but for future-proff I'd like to enforce a general cleanup before assigning probes any FS discriminator.

[wenlei]

llvm/test/CodeGen/X86/fsafdo_probe.ll

68–69

why is this test change? If anything, I was expecting non-zero -> zero for probe discriminators..

[hoy]

llvm/test/CodeGen/X86/fsafdo_probe.ll

68–69

This is to test the non-zero discriminator can be cleaned up by the first FS discriminator pass.

[wenlei]

llvm/test/CodeGen/X86/fsafdo_probe.ll

68–69

Oh, I see. Can you maybe add a comment? Is the "from 0 -> xxx" covering the check on clean up non-zero discriminator?

[hoy]

llvm/test/CodeGen/X86/fsafdo_probe.ll

68–69

Done.

[hoy]

Updating D150741: [FS-AFDO] Clean up non-zero discriminator for pseudo probes at the first FS discriminator pass.

llvm/include/llvm/CodeGen/MIRFSDiscriminator.h

 #include <cstdint>
 
 namespace llvm {
 class MachineFunction;
 
 using namespace sampleprof;
 class MIRAddFSDiscriminators : public MachineFunctionPass {
   MachineFunction *MF = nullptr;
+  FSDiscriminatorPass Pass;
   unsigned LowBit;
   unsigned HighBit;
 
 public:
   static char ID;
   /// PassNum is the sequence number this pass is called, start from 1.
   MIRAddFSDiscriminators(FSDiscriminatorPass P = FSDiscriminatorPass::Pass1)
-      : MachineFunctionPass(ID) {
+      : MachineFunctionPass(ID), Pass(P) {
     LowBit = getFSPassBitBegin(P);
     HighBit = getFSPassBitEnd(P);
     assert(LowBit < HighBit && "HighBit needs to be greater than Lowbit");
   }
 
   StringRef getPassName() const override {
     return "Add FS discriminators in MIR";
   }

llvm/lib/CodeGen/MIRFSDiscriminator.cpp

for (MachineInstr &I : BB) {
         continue;
 
       // Use the id of pseudo probe to compute the discriminator.
       unsigned LineNo =
           I.isPseudoProbe() ? I.getOperand(1).getImm() : DIL->getLine();
       if (LineNo == 0)
         continue;
       unsigned Discriminator = DIL->getDiscriminator();
+      // Clean up discriminators for pseudo probes at the first FS discriminator
+      // pass as their discriminators should not ever be used.
+      if ((Pass == FSDiscriminatorPass::Pass1) && I.isPseudoProbe()) {
+        Discriminator = 0;
+        I.setDebugLoc(DIL->cloneWithDiscriminator(0));
+      }
       uint64_t CallStackHashVal = 0;
       if (ImprovedFSDiscriminator)
         CallStackHashVal = getCallStackHash(DIL);
 
       LocationDiscriminator LD{DIL->getFilename(), LineNo, Discriminator,
                                CallStackHashVal};
       auto &BBMap = LDBM[LD];
       auto R = BBMap.insert(&BB);

llvm/test/CodeGen/X86/fsafdo_probe.ll

 !1 = !DIFile(filename: "foo.c", directory: "b/")
 !2 = !{}
 !3 = !{i32 2, !"Dwarf Version", i32 4}
 !4 = !{i32 2, !"Debug Info Version", i32 3}
 !5 = !{}
 !6 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 5, type: !7, isLocal: false, isDefinition: true, scopeLine: 5, flags: DIFlagPrototyped, isOptimized: true, unit: !0, retainedNodes: !2)
 !7 = !DISubroutineType(types: !2)
 !8 = !DILocation(line: 7, column: 15, scope: !9)
-!9 = !DILexicalBlockFile(scope: !6, file: !1, discriminator: 0)
+;; The discriminator with value 2 is to test that it can be cleaned up by the first FS discriminator pass.
+!9 = !DILexicalBlockFile(scope: !6, file: !1, discriminator: 2)
 !10 = !DILocation(line: 7, column: 3, scope: !9)
 !11 = !DILocation(line: 9, column: 5, scope: !9)
 !12 = !DILocation(line: 14, column: 3, scope: !6)
 !13 = !{i64 6699318081062747564, i64 138464321060, !"foo"}