[AsmPrinter] Emit PCs into requested PCSections

[melver]

Summary

Interpret MD_pcsections in AsmPrinter emitting the requested metadata to
the associated sections. Functions and normal instructions are handled.

Depends on D130886

[dvyukov]

llvm/include/llvm/CodeGen/AsmPrinter.h

195

If metadata is used, is the SmallVector actually expected to be small (<=8)?
I assume that SmallVector<8> will allocate space for 8 statically and then waste it if the vector is larger.

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

1439

Can the other models have pointer size 8?
I assume something like Tiny/Small can have pointer size <4 (?).
Maybe it makes sense to just use getPointerSize() always?

[melver]

llvm/include/llvm/CodeGen/AsmPrinter.h

195

Depends on current usage. If almost all instructions in a functions end up with !pcsections, then it's likely >=8. But currently we're only attaching this to atomics, and I assume the number of atomic ops is low in a function.

But the fact that !pcsections is currently only attached to atomics is something that AsmPrinter really shouldn't care about.

So the choice of 8 is quite arbitrary. SmallVector documentation says to just leave out the small-size, and it'll choose a reasonable default. Let's try that.

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

1439

The code model doesn't have anything to do with target bitness.

Yes, tiny and small CMs can have 64-bit pointers (small is usually the default).
I don't think we have to worry about 16-bit architectures or 16-bit pointers.

The thing we're optimizing is saving space if we know that the code model doesn't allow addressing (relatively) code or data more than 4 GiB away - even on 64-bit architectures.

The different encoding for different CMs is documented in PCSectionsMetadata.rst.

(The idea to use 32-bit relative relocations was pointed out by @pcc. The fact we should distinguish code models was pointed out by @MaskRay. Adding both as reviewers.)

[melver]

Let SmallVector choose small-size.

[MaskRay]

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

1425

prefer const reference if non-null

1449

What does Prev = Sec; do?

1486

omit braces for one-line single statements

1490

omit braces for one-line single statements

1490

As of this patch, no test exercises this line.

If you add tests to later patches, I think they should be moved here.

[MaskRay]

I think the patches are overly splitted. I'd merge the 3 patches before [AsmPrinter] Emit PCs into requested PCSections into this patch.

[melver]

In D130879#3695546, @MaskRay wrote:

I think the patches are overly splitted. I'd merge the 3 patches before [AsmPrinter] Emit PCs into requested PCSections into this patch.

See my proposal. I prefer erring on the side of splitting more, especially independent changes as it helps bisectability.

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

1449

Avoid switching the section if it has already been switched from a previous iteration, reducing overall asm output if we have lots of !pcsection'd instructions in the same function.

The comment above mentions that "short-circuiting the common case where the current section is still valid". If the comment could be clearer, let me know.

1490

It's a chicken-egg problem. We need ISel support for the test to propagate !pcsections through MachineInstrs. So either I add the test here and it'll be broken, or I add it later.

The patch series first adds the new metadata and then AsmPrinter support, and then builds up the plumbing to propagate the metadata. The tests will only start working later.

Based on your suggestion in "[Metadata] Introduce MD_pcsections" I wanted to move the test later. But that seems to contradict your suggestion here.

My proposal is to

• move this patch after "[GlobalISel] Propagate PCSections metadata to MachineInstr" and
• move the test to this patch;
• squash the MCObjectFileInfo changes into this patch;
• keep "[MachineInstr] Allow setting PCSections in ExtraInfo" because it's a dependency for the ISel changes;
• keep "[Metadata] Introduce MD_pcsections" because it's needed for the ISel changes.

[melver]

Address comments.

[melver]

• Change MDNode format to take aux constants as another MDNode.
• Change createPCSections interface.

[melver]

Rebase.

[melver]

PTAL.

@MaskRay - kindly check if I addressed your earlier comments.

Thanks!

llvm/include/llvm/CodeGen/AsmPrinter.h

 // asm writers.  This class primarily handles common functionality used by
 // all asm writers.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CODEGEN_ASMPRINTER_H
 #define LLVM_CODEGEN_ASMPRINTER_H
 
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/CodeGen/AsmPrinterHandler.h"
 #include "llvm/CodeGen/DwarfStringPoolEntry.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/Support/ErrorHandling.h"
private:
   void *GCMetadataPrinters = nullptr; // Really a DenseMap.
 
   /// Emit comments in assembly output if this is true.
   bool VerboseAsm;
 
   /// Output stream for the stack usage file (i.e., .su file).
   std::unique_ptr<raw_fd_ostream> StackUsageStream;
 
+  /// List of symbols to be inserted into PC sections.
+  DenseMap<const MDNode *, SmallVector<const MCSymbol *>> PCSectionsSymbols;
+
   static char ID;
 
 protected:
   MCSymbol *CurrentFnBegin = nullptr;
 
   /// A vector of all debug/EH info emitters we should use. This vector
   /// maintains ownership of the emitters.
   std::vector<HandlerInfo> Handlers;
public:
   void emitStackUsage(const MachineFunction &MF);
 
   void emitBBAddrMapSection(const MachineFunction &MF);
 
   void emitPseudoProbe(const MachineInstr &MI);
 
   void emitRemarksSection(remarks::RemarkStreamer &RS);
 
+  /// Emits a label as reference for PC sections.
+  void emitPCSectionsLabel(const MachineFunction &MF, const MDNode *MD);
+
+  /// Emits the PC sections collected from instructions.
+  void emitPCSections(const MachineFunction &MF);
+
   /// Get the CFISection type for a function.
   CFISection getFunctionCFISectionType(const Function &F) const;
 
   /// Get the CFISection type for a function.
   CFISection getFunctionCFISectionType(const MachineFunction &MF) const;
 
   /// Get the CFISection type for the module.
   CFISection getModuleCFISectionType() const { return ModuleCFISection; }

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

void AsmPrinter::emitStackUsage(const MachineFunction &MF) {
 
   *StackUsageStream << ':' << MF.getName() << '\t' << StackSize << '\t';
   if (FrameInfo.hasVarSizedObjects())
     *StackUsageStream << "dynamic\n";
   else
     *StackUsageStream << "static\n";
 }
 
-static bool needFuncLabelsForEHOrDebugInfo(const MachineFunction &MF) {
+void AsmPrinter::emitPCSectionsLabel(const MachineFunction &MF,
+                                     const MDNode *MD) {
+  MCSymbol *S = MF.getContext().createTempSymbol("pcsection");
+  OutStreamer->emitLabel(S);
+  PCSectionsSymbols[MD].emplace_back(S);
+}
+
+void AsmPrinter::emitPCSections(const MachineFunction &MF) {
+  const Function &F = MF.getFunction();
+  if (PCSectionsSymbols.empty() && !F.hasMetadata(LLVMContext::MD_pcsections))
+    return;
+
+  const CodeModel::Model CM = MF.getTarget().getCodeModel();
+  const unsigned RelativeRelocSize =
+      (CM == CodeModel::Medium || CM == CodeModel::Large) ? getPointerSize()
+                                                          : 4;
+
+  // Switch to PCSection, short-circuiting the common case where the current
+  // section is still valid (assume most MD_pcsections contain just 1 section).
+  auto SwitchSection = [&, Prev = StringRef()](const StringRef &Sec) mutable {
+    if (Sec == Prev)
+      return;
+    MCSection *S = getObjFileLowering().getPCSection(Sec, MF.getSection());
+    assert(S && "PC section is not initialized");
+    OutStreamer->switchSection(S);
+    Prev = Sec;
+  };
+  // Emit symbols into sections and data as specified in the pcsections MDNode.
+  auto EmitForMD = [&](const MDNode &MD, ArrayRef<const MCSymbol *> Syms,
+                       bool Deltas) {
+    // Expect the first operand to be a section name. After that, any number of
+    // constants may appear, which will simply be emitted into the current
+    // section (the user of MD_pcsections decides the format of encoded data).
+    assert(isa<MDString>(MD.getOperand(0)) && "first operand not a string");
+
+    for (const MDOperand &MDO : MD.operands()) {
+      if (auto *C = dyn_cast<ConstantAsMetadata>(MDO)) {
+        emitGlobalConstant(F.getParent()->getDataLayout(), C->getValue());
+      } else {
+        SwitchSection(cast<MDString>(MDO)->getString());
+        const MCSymbol *Prev = Syms.front();
+        for (const MCSymbol *Sym : Syms) {
+          if (Sym == Prev || !Deltas) {
+            // Use the entry itself as the base of the relative offset.
+            MCSymbol *Base = MF.getContext().createTempSymbol("pcsection_base");
+            OutStreamer->emitLabel(Base);
+            // Emit relative relocation `addr - base`, which avoids a dynamic
+            // relocation in the final binary. User will get the address with
+            // `base + addr`.
+            emitLabelDifference(Sym, Base, RelativeRelocSize);
+          } else {
+            emitLabelDifference(Sym, Prev, 4);
+          }
+          Prev = Sym;
+        }
+      }
+    }
+  };
+
+  OutStreamer->pushSection();
+  // Emit PCs for function start and function size.
+  if (const MDNode *MD = F.getMetadata(LLVMContext::MD_pcsections)) {
+    EmitForMD(*MD, {getFunctionBegin(), getFunctionEnd()}, true);
+  }
+  // Emit PCs for instructions collected.
+  for (const auto &MS : PCSectionsSymbols) {
+    EmitForMD(*MS.first, MS.second, false);
+  }
+  OutStreamer->popSection();
+  PCSectionsSymbols.clear();
+}
+
+/// Returns true if function begin and end labels should be emitted.
+static bool needFuncLabels(const MachineFunction &MF) {
   MachineModuleInfo &MMI = MF.getMMI();
-  if (!MF.getLandingPads().empty() || MF.hasEHFunclets() || MMI.hasDebugInfo())
+  if (!MF.getLandingPads().empty() || MF.hasEHFunclets() ||
+      MMI.hasDebugInfo() ||
+      MF.getFunction().hasMetadata(LLVMContext::MD_pcsections))
     return true;
 
   // We might emit an EH table that uses function begin and end labels even if
   // we don't have any landingpads.
   if (!MF.getFunction().hasPersonalityFn())
     return false;
   return !isNoOpWithoutInvoke(
       classifyEHPersonality(MF.getFunction().getPersonalityFn()));
for (auto &MI : MBB) {
         HasAnyRealCode = true;
         ++NumInstsInFunction;
       }
 
       // If there is a pre-instruction symbol, emit a label for it here.
       if (MCSymbol *S = MI.getPreInstrSymbol())
         OutStreamer->emitLabel(S);
 
+      if (MDNode *MD = MI.getPCSections())
+        emitPCSectionsLabel(*MF, MD);
+
       for (const HandlerInfo &HI : Handlers) {
         NamedRegionTimer T(HI.TimerName, HI.TimerDescription, HI.TimerGroupName,
                            HI.TimerGroupDescription, TimePassesIsEnabled);
         HI.Handler->beginInstruction(&MI);
       }
 
       if (isVerbose())
         emitComments(MI, OutStreamer->getCommentOS());
if (Sym->isDefined())
       continue;
     OutStreamer->AddComment("Address of block that was removed by CodeGen");
     OutStreamer->emitLabel(Sym);
   }
 
   // Emit target-specific gunk after the function body.
   emitFunctionBodyEnd();
 
-  if (needFuncLabelsForEHOrDebugInfo(*MF) ||
-      MAI->hasDotTypeDotSizeDirective()) {
+  if (needFuncLabels(*MF) || MAI->hasDotTypeDotSizeDirective()) {
     // Create a symbol for the end of function.
     CurrentFnEnd = createTempSymbol("func_end");
     OutStreamer->emitLabel(CurrentFnEnd);
   }
 
   // If the target wants a .size directive for the size of the function, emit
   // it.
   if (MAI->hasDotTypeDotSizeDirective()) {
for (const HandlerInfo &HI : Handlers) {
     HI.Handler->endFunction(MF);
   }
 
   // Emit section containing BB address offsets and their metadata, when
   // BB labels are requested for this function. Skip empty functions.
   if (MF->hasBBLabels() && HasAnyRealCode)
     emitBBAddrMapSection(*MF);
 
+  // Emit sections containing instruction and function PCs.
+  emitPCSections(*MF);
+
   // Emit section containing stack size metadata.
   emitStackSizeSection(*MF);
 
   // Emit .su file containing function stack size information.
   emitStackUsage(*MF);
 
   emitPatchableFunctionEntries();
 
void AsmPrinter::SetupMachineFunction(MachineFunction &MF) {
   CurrentFnBegin = nullptr;
   CurrentSectionBeginSym = nullptr;
   MBBSectionRanges.clear();
   MBBSectionExceptionSyms.clear();
   bool NeedsLocalForSize = MAI->needsLocalForSize();
   if (F.hasFnAttribute("patchable-function-entry") ||
       F.hasFnAttribute("function-instrument") ||
       F.hasFnAttribute("xray-instruction-threshold") ||
-      needFuncLabelsForEHOrDebugInfo(MF) || NeedsLocalForSize ||
+      needFuncLabels(MF) || NeedsLocalForSize ||
       MF.getTarget().Options.EmitStackSizeSection || MF.hasBBLabels()) {
     CurrentFnBegin = createTempSymbol("func_begin");
     if (NeedsLocalForSize)
       CurrentFnSymForSize = CurrentFnBegin;
   }
 
   ORE = &getAnalysis<MachineOptimizationRemarkEmitterPass>().getORE();
 }