diff --git a/llvm/include/llvm/IR/PseudoProbe.h b/llvm/include/llvm/IR/PseudoProbe.h --- a/llvm/include/llvm/IR/PseudoProbe.h +++ b/llvm/include/llvm/IR/PseudoProbe.h @@ -24,8 +24,15 @@ constexpr const char *PseudoProbeDescMetadataName = "llvm.pseudo_probe_desc"; +enum class PseudoProbeReservedId { Invalid = 0, Last = Invalid }; + enum class PseudoProbeType { Block = 0, IndirectCall, DirectCall }; +enum class PseudoProbeAttributes { + Reserved = 0x1, + Sentinel = 0x2, // A place holder for function entry address. +}; + // The saturated distrution factor representing 100% for block probes. constexpr static uint64_t PseudoProbeFullDistributionFactor = std::numeric_limits::max(); @@ -80,6 +87,10 @@ float Factor; }; +static inline bool isSentinelProbe(uint32_t Flags) { + return Flags & (uint32_t)PseudoProbeAttributes::Sentinel; +} + Optional extractProbe(const Instruction &Inst); void setProbeDistributionFactor(Instruction &Inst, float Factor); diff --git a/llvm/include/llvm/MC/MCStreamer.h b/llvm/include/llvm/MC/MCStreamer.h --- a/llvm/include/llvm/MC/MCStreamer.h +++ b/llvm/include/llvm/MC/MCStreamer.h @@ -1105,7 +1105,8 @@ /// Emit the a pseudo probe into the current section. virtual void emitPseudoProbe(uint64_t Guid, uint64_t Index, uint64_t Type, uint64_t Attr, - const MCPseudoProbeInlineStack &InlineStack); + const MCPseudoProbeInlineStack &InlineStack, + MCSymbol *FnSym); /// Set the bundle alignment mode from now on in the section. /// The argument is the power of 2 to which the alignment is set. The diff --git a/llvm/include/llvm/Transforms/IPO/SampleProfileProbe.h b/llvm/include/llvm/Transforms/IPO/SampleProfileProbe.h --- a/llvm/include/llvm/Transforms/IPO/SampleProfileProbe.h +++ b/llvm/include/llvm/Transforms/IPO/SampleProfileProbe.h @@ -41,8 +41,6 @@ pair_hash>; using FuncProbeFactorMap = StringMap; -enum class PseudoProbeReservedId { Invalid = 0, Last = Invalid }; - class PseudoProbeDescriptor { uint64_t FunctionGUID; uint64_t FunctionHash; diff --git a/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp --- a/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/PseudoProbePrinter.cpp @@ -48,5 +48,6 @@ } SmallVector InlineStack(llvm::reverse(ReversedInlineStack)); - Asm->OutStreamer->emitPseudoProbe(Guid, Index, Type, Attr, InlineStack); + Asm->OutStreamer->emitPseudoProbe(Guid, Index, Type, Attr, InlineStack, + Asm->CurrentFnSym); } diff --git a/llvm/lib/CodeGen/PseudoProbeInserter.cpp b/llvm/lib/CodeGen/PseudoProbeInserter.cpp --- a/llvm/lib/CodeGen/PseudoProbeInserter.cpp +++ b/llvm/lib/CodeGen/PseudoProbeInserter.cpp @@ -51,7 +51,7 @@ if (!ShouldRun) return false; const TargetInstrInfo *TII = MF.getSubtarget().getInstrInfo(); - bool Changed = false; + uint64_t FuncGUID = getFuncGUID(MF); for (MachineBasicBlock &MBB : MF) { MachineInstr *FirstInstr = nullptr; for (MachineInstr &MI : MBB) { @@ -69,7 +69,6 @@ PseudoProbeDwarfDiscriminator::extractProbeType(Value)) .addImm(PseudoProbeDwarfDiscriminator::extractProbeAttributes( Value)); - Changed = true; } } } @@ -101,7 +100,6 @@ auto *ProbeInstr = &*Cur; MBB.remove(ProbeInstr); MBB.insert(FirstInstr, ProbeInstr); - Changed = true; } } else { // Probes not surrounded by any real instructions in the same block are @@ -118,15 +116,63 @@ for (auto *MI : ToBeRemoved) MI->eraseFromParent(); + } + + // Remove probes with broken inline contexts. Such probes will cause + // encoding trouble and will also cause misleading profile genereated. + for (MachineInstr &MI : make_early_inc_range(MBB)) { + if (!MI.isPseudoProbe()) + continue; + uint64_t GUID = MI.getOperand(0).getImm(); + if (GUID == FuncGUID) + continue; + + if (!MI.getDebugLoc()) { + MI.eraseFromParent(); + continue; + } - Changed |= !ToBeRemoved.empty(); + auto *InlinedAt = MI.getDebugLoc().getInlinedAt(); + if (!InlinedAt) { + MI.eraseFromParent(); + continue; + } + + while (InlinedAt->getInlinedAt()) + InlinedAt = InlinedAt->getInlinedAt(); + if (InlinedAt->getScope()->getSubprogram() != + MF.getFunction().getSubprogram()) + MI.eraseFromParent(); + } + + // Add a fake sentinel probe to the entry of the function. The probe + // serves as a place holder of the current function address and will be + // used for offset-based probe enocoding. The probe won't be actually + // encoded in the binary. + if (MBB.isEntryBlock()) { + BuildMI(MBB, MBB.begin(), DebugLoc(), + TII->get(TargetOpcode::PSEUDO_PROBE)) + .addImm(FuncGUID) + .addImm((uint32_t)PseudoProbeReservedId::Invalid) + .addImm((uint32_t)PseudoProbeType::Block) + .addImm((uint32_t)PseudoProbeAttributes::Sentinel); } } - return Changed; + return true; } private: + uint64_t getFuncGUID(MachineFunction &MF) { + StringRef Name = MF.getName(); + if (const DISubprogram *SP = MF.getFunction().getSubprogram()) { + Name = SP->getLinkageName(); + if (Name.empty()) + Name = SP->getName(); + } + return Function::getGUID(Name); + } + uint64_t getFuncGUID(Module *M, DILocation *DL) { auto *SP = DL->getScope()->getSubprogram(); auto Name = SP->getLinkageName(); diff --git a/llvm/lib/MC/MCAsmStreamer.cpp b/llvm/lib/MC/MCAsmStreamer.cpp --- a/llvm/lib/MC/MCAsmStreamer.cpp +++ b/llvm/lib/MC/MCAsmStreamer.cpp @@ -378,7 +378,7 @@ void emitPseudoProbe(uint64_t Guid, uint64_t Index, uint64_t Type, uint64_t Attr, - const MCPseudoProbeInlineStack &InlineStack) override; + const MCPseudoProbeInlineStack &InlineStack, MCSymbol *FnSym) override; void emitBundleAlignMode(unsigned AlignPow2) override; void emitBundleLock(bool AlignToEnd) override; @@ -2338,13 +2338,17 @@ void MCAsmStreamer::emitPseudoProbe( uint64_t Guid, uint64_t Index, uint64_t Type, uint64_t Attr, - const MCPseudoProbeInlineStack &InlineStack) { + const MCPseudoProbeInlineStack &InlineStack, MCSymbol *FnSym) { OS << "\t.pseudoprobe\t" << Guid << " " << Index << " " << Type << " " << Attr; // Emit inline stack like // @ GUIDmain:3 @ GUIDCaller:1 @ GUIDDirectCaller:11 for (const auto &Site : InlineStack) OS << " @ " << std::get<0>(Site) << ":" << std::get<1>(Site); + + if (isSentinelProbe(Attr)) + OS << " " << FnSym->getName(); + EmitEOL(); } diff --git a/llvm/lib/MC/MCParser/AsmParser.cpp b/llvm/lib/MC/MCParser/AsmParser.cpp --- a/llvm/lib/MC/MCParser/AsmParser.cpp +++ b/llvm/lib/MC/MCParser/AsmParser.cpp @@ -5904,10 +5904,21 @@ InlineStack.push_back(Site); } + // Parse function entry name for fake entry probe + MCSymbol *FnSym = nullptr; + if (isSentinelProbe(Attr)) { + if (getLexer().is(AsmToken::Identifier)) { + StringRef FnName; + if (parseIdentifier(FnName)) + return Error(getLexer().getLoc(), "unexpected token in '.pseudoprobe' directive"); + FnSym = getContext().lookupSymbol(FnName); + } + } + if (parseEOL()) return true; - getStreamer().emitPseudoProbe(Guid, Index, Type, Attr, InlineStack); + getStreamer().emitPseudoProbe(Guid, Index, Type, Attr, InlineStack, FnSym); return false; } diff --git a/llvm/lib/MC/MCPseudoProbe.cpp b/llvm/lib/MC/MCPseudoProbe.cpp --- a/llvm/lib/MC/MCPseudoProbe.cpp +++ b/llvm/lib/MC/MCPseudoProbe.cpp @@ -43,6 +43,9 @@ void MCPseudoProbe::emit(MCObjectStreamer *MCOS, const MCPseudoProbe *LastProbe) const { + assert(!isSentinelProbe(getAttributes()) && + "Fake entry probe should not come here"); + // Emit Index MCOS->emitULEB128IntValue(Index); // Emit Type and the flag: @@ -53,23 +56,17 @@ assert(Attributes <= 0x7 && "Probe attributes too big to encode, exceeding 7"); uint8_t PackedType = Type | (Attributes << 4); - uint8_t Flag = LastProbe ? ((int8_t)MCPseudoProbeFlag::AddressDelta << 7) : 0; + uint8_t Flag = (int8_t)MCPseudoProbeFlag::AddressDelta << 7; MCOS->emitInt8(Flag | PackedType); - if (LastProbe) { - // Emit the delta between the address label and LastProbe. - const MCExpr *AddrDelta = - buildSymbolDiff(MCOS, Label, LastProbe->getLabel()); - int64_t Delta; - if (AddrDelta->evaluateAsAbsolute(Delta, MCOS->getAssemblerPtr())) { - MCOS->emitSLEB128IntValue(Delta); - } else { - MCOS->insert(new MCPseudoProbeAddrFragment(AddrDelta)); - } + // Emit the delta between the address label and LastProbe. + assert(LastProbe && "Last probe should not be null"); + const MCExpr *AddrDelta = buildSymbolDiff(MCOS, Label, LastProbe->getLabel()); + int64_t Delta; + if (AddrDelta->evaluateAsAbsolute(Delta, MCOS->getAssemblerPtr())) { + MCOS->emitSLEB128IntValue(Delta); } else { - // Emit label as a symbolic code address. - MCOS->emitSymbolValue( - Label, MCOS->getContext().getAsmInfo()->getCodePointerSize()); + MCOS->insert(new MCPseudoProbeAddrFragment(AddrDelta)); } LLVM_DEBUG({ @@ -137,12 +134,19 @@ // Emit Guid MCOS->emitInt64(Guid); // Emit number of probes in this node - MCOS->emitULEB128IntValue(Probes.size()); + uint64_t NumProbes = 0; + for (const auto &Probe : Probes) { + if (!isSentinelProbe(Probe.getAttributes())) + NumProbes++; + } + + MCOS->emitULEB128IntValue(NumProbes); // Emit number of direct inlinees MCOS->emitULEB128IntValue(Children.size()); // Emit probes in this group for (const auto &Probe : Probes) { - Probe.emit(MCOS, LastProbe); + if (!isSentinelProbe(Probe.getAttributes())) + Probe.emit(MCOS, LastProbe); LastProbe = &Probe; } } else { diff --git a/llvm/lib/MC/MCStreamer.cpp b/llvm/lib/MC/MCStreamer.cpp --- a/llvm/lib/MC/MCStreamer.cpp +++ b/llvm/lib/MC/MCStreamer.cpp @@ -1102,14 +1102,19 @@ void MCStreamer::emitPseudoProbe(uint64_t Guid, uint64_t Index, uint64_t Type, uint64_t Attr, - const MCPseudoProbeInlineStack &InlineStack) { + const MCPseudoProbeInlineStack &InlineStack, + MCSymbol *FnSym) { auto &Context = getContext(); - - // Create a symbol at in the current section for use in the probe. - MCSymbol *ProbeSym = Context.createTempSymbol(); - - // Set the value of the symbol to use for the MCPseudoProbe. - emitLabel(ProbeSym); + MCSymbol *ProbeSym; + + if (isSentinelProbe(Attr)) { + ProbeSym = FnSym; + } else { + // Create a symbol at in the current section for use in the probe. + ProbeSym = Context.createTempSymbol(); + // Set the value of the symbol to use for the MCPseudoProbe. + emitLabel(ProbeSym); + } // Create a (local) probe entry with the symbol. MCPseudoProbe Probe(ProbeSym, Guid, Index, Type, Attr); diff --git a/llvm/test/Transforms/SampleProfile/pseudo-probe-emit-inline.ll b/llvm/test/Transforms/SampleProfile/pseudo-probe-emit-inline.ll --- a/llvm/test/Transforms/SampleProfile/pseudo-probe-emit-inline.ll +++ b/llvm/test/Transforms/SampleProfile/pseudo-probe-emit-inline.ll @@ -12,14 +12,16 @@ define dso_local void @foo2() !dbg !7 { ; CHECK-IL: call void @llvm.pseudoprobe(i64 [[#GUID1:]], i64 1, i32 0, i64 -1), !dbg ![[#]] -; CHECK-ASM: .pseudoprobe [[#GUID1:]] 1 0 0 +; CHECK-ASM: .pseudoprobe [[#GUID1:]] 0 0 2 foo2 +; CHECK-ASM: .pseudoprobe [[#GUID1]] 1 0 0 ret void, !dbg !10 } define dso_local void @foo() #0 !dbg !11 { ; CHECK-IL: call void @llvm.pseudoprobe(i64 [[#GUID2:]], i64 1, i32 0, i64 -1), !dbg ![[#]] ; CHECK-IL: call void @llvm.pseudoprobe(i64 [[#GUID1]], i64 1, i32 0, i64 -1), !dbg ![[#DL1:]] -; CHECK-ASM: .pseudoprobe [[#GUID2:]] 1 0 0 +; CHECK-ASM: .pseudoprobe [[#GUID2:]] 0 0 2 foo +; CHECK-ASM: .pseudoprobe [[#GUID2]] 1 0 0 ; CHECK-ASM: .pseudoprobe [[#GUID1]] 1 0 0 @ [[#GUID2]]:2 call void @foo2(), !dbg !12 ret void, !dbg !13 @@ -29,7 +31,8 @@ ; CHECK-IL: call void @llvm.pseudoprobe(i64 [[#GUID3:]], i64 1, i32 0, i64 -1), !dbg ![[#]] ; CHECK-IL: call void @llvm.pseudoprobe(i64 [[#GUID2]], i64 1, i32 0, i64 -1), !dbg ![[#DL2:]] ; CHECK-IL: call void @llvm.pseudoprobe(i64 [[#GUID1]], i64 1, i32 0, i64 -1), !dbg ![[#DL3:]] -; CHECK-ASM: .pseudoprobe [[#GUID3:]] 1 0 0 +; CHECK-ASM: .pseudoprobe [[#GUID3:]] 0 0 2 entry +; CHECK-ASM: .pseudoprobe [[#GUID3]] 1 0 0 ; CHECK-ASM: .pseudoprobe [[#GUID2]] 1 0 0 @ [[#GUID3]]:2 ; CHECK-ASM: .pseudoprobe [[#GUID1]] 1 0 0 @ [[#GUID3]]:2 @ [[#GUID2]]:2 call void @foo(), !dbg !18 @@ -71,6 +74,7 @@ ; CHECK-OBJ: .pseudo_probe_desc ; CHECK-OBJ: .pseudo_probe +; CHECK-OBJ-NOT: .rela.pseudo_probe !llvm.dbg.cu = !{!0} !llvm.module.flags = !{!3, !4} diff --git a/llvm/test/Transforms/SampleProfile/pseudo-probe-emit.ll b/llvm/test/Transforms/SampleProfile/pseudo-probe-emit.ll --- a/llvm/test/Transforms/SampleProfile/pseudo-probe-emit.ll +++ b/llvm/test/Transforms/SampleProfile/pseudo-probe-emit.ll @@ -17,8 +17,10 @@ bb0: %cmp = icmp eq i32 %x, 0 ; CHECK-IL: call void @llvm.pseudoprobe(i64 [[#GUID:]], i64 1, i32 0, i64 -1), !dbg ![[#FAKELINE:]] -; CHECK-MIR: PSEUDO_PROBE [[#GUID:]], 1, 0, 0 -; CHECK-ASM: .pseudoprobe [[#GUID:]] 1 0 0 +; CHECK-MIR: PSEUDO_PROBE [[#GUID:]], 0, 0, 2 +; CHECK-MIR: PSEUDO_PROBE [[#GUID]], 1, 0, 0 +; CHECK-ASM: .pseudoprobe [[#GUID:]] 0 0 2 foo +; CHECK-ASM: .pseudoprobe [[#GUID]] 1 0 0 br i1 %cmp, label %bb1, label %bb2 bb1: @@ -44,13 +46,15 @@ ret void, !dbg !12 } -declare void @bar(i32 %x) +declare void @bar(i32 %x) define internal void @foo2(ptr %f) !dbg !4 { entry: ; CHECK-IL: call void @llvm.pseudoprobe(i64 [[#GUID2:]], i64 1, i32 0, i64 -1) -; CHECK-MIR: PSEUDO_PROBE [[#GUID2:]], 1, 0, 0 -; CHECK-ASM: .pseudoprobe [[#GUID2:]] 1 0 0 +; CHECK-MIR: PSEUDO_PROBE [[#GUID2:]], 0, 0, 2 +; CHECK-MIR: PSEUDO_PROBE [[#GUID2]], 1, 0, 0 +; CHECK-ASM: .pseudoprobe [[#GUID2:]] 0 0 2 foo2 +; CHECK-ASM: .pseudoprobe [[#GUID2]] 1 0 0 ; Check pseudo_probe metadata attached to the indirect call instruction. ; CHECK-IL: call void %f(i32 1), !dbg ![[#PROBE0:]] ; CHECK-MIR: PSEUDO_PROBE [[#GUID2]], 2, 1, 0 @@ -92,7 +96,8 @@ ; CHECK-ASM-NEXT: .ascii "foo2" ; CHECK-OBJ-COUNT-2: .pseudo_probe_desc -; CHECK-OBJ-COUNT-2: .pseudo_probe +; CHECK-OBJ: .pseudo_probe +; CHECK-OBJ-NOT: .rela.pseudo_probe !llvm.dbg.cu = !{!0} !llvm.module.flags = !{!9, !10}