[PATCH] D26412: [XRay] Support AArch64 in LLVM

[Dean Michael Berris via llvm-commits]

dberris added inline comments.


================
Comment at: lib/Target/AArch64/AArch64AsmPrinter.cpp:159
+      "xray_instr_map", ELF::SHT_PROGBITS,
+      ELF::SHF_ALLOC | ELF::SHF_GROUP | ELF::SHF_MERGE, 0,
+      CurrentFnSym->getName());
----------------
rSerge wrote:
> dberris wrote:
> > I distinctly remember having to do something special for non-COMDAT sections, especially when doing section-per-function in x86. I suspect you'd want to replicate that logic here somehow, with a `//TODO` to merge the logic for ELF XRay sleds at a higher level for better re-use.
> Yes, I see it now. Shall this code be CPU-independent? If so, let me look if I can refactor it in this patch...
> The CPU-independent code seems to be
> 
> ```
>   if (Sleds.empty())
>     return;
>   if (Subtarget->isTargetELF()) {
>     auto PrevSection = OutStreamer->getCurrentSectionOnly();
>     auto Fn = MF->getFunction();
>     MCSection *Section = nullptr;
>     if (Fn->hasComdat()) {
>       Section = OutContext.getELFSection("xray_instr_map", ELF::SHT_PROGBITS,
>                                          ELF::SHF_ALLOC | ELF::SHF_GROUP, 0,
>                                          Fn->getComdat()->getName());
>     } else {
>       Section = OutContext.getELFSection("xray_instr_map", ELF::SHT_PROGBITS,
>                                          ELF::SHF_ALLOC);
>     }
> 
>     // Before we switch over, we force a reference to a label inside the
>     // xray_instr_map section. Since EmitXRayTable() is always called just
>     // before the function's end, we assume that this is happening after the
>     // last return instruction.
>     //
>     // We then align the reference to 16 byte boundaries, which we determined
>     // experimentally to be beneficial to avoid causing decoder stalls.
>     MCSymbol *Tmp = OutContext.createTempSymbol("xray_synthetic_", true);
>     OutStreamer->EmitCodeAlignment(16);
>     OutStreamer->EmitSymbolValue(Tmp, 8, false);
>     OutStreamer->SwitchSection(Section);
>     OutStreamer->EmitLabel(Tmp);
>     for (const auto &Sled : Sleds) {
>          // CPU-dependent code block here (depends of 64- or 32- bit word width)
>     }
>     OutStreamer->SwitchSection(PrevSection);
>   }
>   Sleds.clear();
> }
> ```
Yep, I think you found it. Would be happy to wait for the refactoring to make this simpler moving forward when we cover more architectures.


https://reviews.llvm.org/D26412